From 15b0148542ba3529e6bf75401f556feb79b2a6ec Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Wed, 4 Jun 2025 13:01:58 +0200 Subject: [PATCH] Fix stacktrace for missing database connection - Add error handling for database connection failures - Suppress stacktrace and add error messages for user and admin - Improve user experience with clear error messages --- application/controllers/ErrorController.php | 27 ++++++---- .../controllers/MigrationsController.php | 49 +++++++++++++---- .../controllers/MyDevicesController.php | 53 ++++++++++++++++--- .../Config/General/ApplicationConfigForm.php | 12 +++++ application/forms/PreferenceForm.php | 13 ++++- application/views/scripts/about/index.phtml | 39 +++++++++++--- library/Icinga/Web/StyleSheet.php | 1 + public/css/icinga/db-connection-warning.less | 32 +++++++++++ 8 files changed, 189 insertions(+), 37 deletions(-) create mode 100644 public/css/icinga/db-connection-warning.less diff --git a/application/controllers/ErrorController.php b/application/controllers/ErrorController.php index 476b71f2e..7ad7ac990 100644 --- a/application/controllers/ErrorController.php +++ b/application/controllers/ErrorController.php @@ -6,6 +6,7 @@ namespace Icinga\Controllers; use Icinga\Application\Hook\DbMigrationHook; use Icinga\Application\MigrationManager; use Icinga\Exception\IcingaException; +use Throwable; use Zend_Controller_Plugin_ErrorHandler; use Icinga\Application\Icinga; use Icinga\Application\Logger; @@ -96,17 +97,23 @@ class ErrorController extends ActionController $mm = MigrationManager::instance(); $action = $this->getRequest()->getActionName(); $controller = $this->getRequest()->getControllerName(); - if ($action !== 'hint' && $controller !== 'migrations' && $mm->hasMigrations($moduleName)) { - // The view renderer from IPL web doesn't render the HTML content set in the respective - // controller if the error_handler request param is set, as it doesn't support error - // rendering. Since this error handler isn't caused by the migrations controller, we can - // safely unset this. - $this->setParam('error_handler', null); - $this->forward('hint', 'migrations', 'default', [ - DbMigrationHook::MIGRATION_PARAM => $moduleName - ]); + $hasPending = false; + try { + $hasPending = $mm->hasPendingMigrations(); + if ($action !== 'hint' && $controller !== 'migrations' && $mm->hasMigrations($moduleName)) { + // The view renderer from IPL web doesn't render the HTML content set in the respective + // controller if the error_handler request param is set, as it doesn't support error + // rendering. Since this error handler isn't caused by the migrations controller, we can + // safely unset this. + $this->setParam('error_handler', null); + $this->forward('hint', 'migrations', 'default', [ + DbMigrationHook::MIGRATION_PARAM => $moduleName + ]); - return; + return; + } + } catch (Throwable $e) { + //suppress } $this->getResponse()->setHttpResponseCode(500); diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index 5229f066c..8c22d7b6d 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -19,6 +19,9 @@ use ipl\Html\HtmlElement; use ipl\Html\Text; use ipl\Web\Compat\CompatController; use ipl\Web\Widget\ActionLink; +use ipl\Web\Widget\Icon; +use ipl\Web\Widget\Link; +use Throwable; class MigrationsController extends CompatController { @@ -55,24 +58,48 @@ class MigrationsController extends CompatController $migrateListForm = new MigrationForm(); $migrateListForm->setAttribute('id', $this->getRequest()->protectId('migration-form')); - $migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges()); + try { + $migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges()); - if ($canApply && $mm->hasPendingMigrations()) { - $migrateAllButton = new SubmitButtonElement(sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [ + if ($canApply && $mm->hasPendingMigrations()) { + $migrateAllButton = new SubmitButtonElement(sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [ 'form' => $migrateListForm->getAttribute('id')->getValue(), 'label' => $this->translate('Migrate All'), 'title' => $this->translate('Migrate all pending migrations') - ]); + ]); - // Is the first button, so will be cloned and that the visible - // button is outside the form doesn't matter for Web's JS - $migrateListForm->registerElement($migrateAllButton); + // Is the first button, so will be cloned and that the visible + // button is outside the form doesn't matter for Web's JS + $migrateListForm->registerElement($migrateAllButton); - // Make sure it looks familiar, even if not inside a form - $migrateAllButton->setWrapper(new HtmlElement('div', Attributes::create(['class' => 'icinga-controls']))); + // Make sure it looks familiar, even if not inside a form + $migrateAllButton->setWrapper( + new HtmlElement('div', Attributes::create(['class' => 'icinga-controls'])) + ); - $this->controls->getAttributes()->add('class', 'default-layout'); - $this->addControl($migrateAllButton); + $this->controls->getAttributes()->add('class', 'default-layout'); + $this->addControl($migrateAllButton); + } + } catch (Throwable $e) { + $this->addContent( + new HtmlElement( + 'div', + new Attributes(['class' => 'db-connection-warning']), + new Icon('warning'), + new HtmlElement('ul', null), + new HtmlElement( + 'p', + null, + new Text( + $this->translate(' No Configuration Database selected. + To establish a valid database connection set the Configuration Database field. ') + ) + ), + new HtmlElement('ul', null), + new Link($this->translate('Configuration Database'), 'config/general/') + ) + ); + return; } $this->handleFormatRequest($mm->toArray()); diff --git a/application/controllers/MyDevicesController.php b/application/controllers/MyDevicesController.php index e0fb98a52..a9e68de7c 100644 --- a/application/controllers/MyDevicesController.php +++ b/application/controllers/MyDevicesController.php @@ -4,10 +4,15 @@ namespace Icinga\Controllers; use Icinga\Common\Database; -use Icinga\Web\Notification; use Icinga\Web\RememberMe; use Icinga\Web\RememberMeUserDevicesList; +use ipl\Html\Attributes; +use ipl\Html\HtmlElement; +use ipl\Html\Text; use ipl\Web\Compat\CompatController; +use ipl\Web\Widget\Icon; +use ipl\Web\Widget\Link; +use Throwable; /** * MyDevicesController @@ -49,6 +54,46 @@ class MyDevicesController extends CompatController public function indexAction() { + try { + $this->getDb(); + } catch (Throwable $e) { + $hasConfigPermission = $this->hasPermission('config/*'); + if ($hasConfigPermission) { + $this->addContent( + new HtmlElement( + 'div', + new Attributes(['class' => 'db-connection-warning']), + new Icon('warning'), + new HtmlElement( + 'p', + null, + new Text($this->translate( + 'No Configuration Database selected.' + . 'To establish a valid database connection set the Configuration Database field.' + )) + ), + new Link($this->translate('Configuration Database'), 'config/general/') + ) + ); + } else { + $this->addContent( + new HtmlElement( + 'div', + new Attributes(['class' => 'db-connection-warning']), + new Icon('warning'), + new HtmlElement( + 'p', + null, + new Text($this->translate( + 'No Configuration Database selected.' + . 'You don`t have permission to change this setting. Please contact an administrator.' + )) + ) + ) + ); + } + return; + } $name = $this->auth->getUser()->getUsername(); $data = (new RememberMeUserDevicesList()) @@ -57,12 +102,6 @@ class MyDevicesController extends CompatController ->setUrl('my-devices/delete'); $this->addContent($data); - - if (! $this->hasDb()) { - Notification::warning( - $this->translate("Users can't stay logged in without a database configuration backend") - ); - } } public function deleteAction() diff --git a/application/forms/Config/General/ApplicationConfigForm.php b/application/forms/Config/General/ApplicationConfigForm.php index b88ea5dfa..1ced994d9 100644 --- a/application/forms/Config/General/ApplicationConfigForm.php +++ b/application/forms/Config/General/ApplicationConfigForm.php @@ -3,9 +3,11 @@ namespace Icinga\Forms\Config\General; +use Icinga\Application\Config; use Icinga\Application\Icinga; use Icinga\Data\ResourceFactory; use Icinga\Web\Form; +use ipl\Html\Text; /** * Configuration form for general application options @@ -100,6 +102,16 @@ class ApplicationConfigForm extends Form ) ); + $config = Config::app()->getSection('global'); + if (!isset($config->config_resource)) { + $missingConfigResource = + Text::create( + $this->translate("No Configuration Database selected. + Please set the field to establish a valid database connection.") + ); + $this->warning($missingConfigResource, false); + } + return $this; } } diff --git a/application/forms/PreferenceForm.php b/application/forms/PreferenceForm.php index 3e431db90..01d4574af 100644 --- a/application/forms/PreferenceForm.php +++ b/application/forms/PreferenceForm.php @@ -17,6 +17,7 @@ use Icinga\Web\Notification; use Icinga\Web\Session; use Icinga\Web\StyleSheet; use ipl\Html\HtmlElement; +use ipl\Html\Text; use ipl\I18n\GettextTranslator; use ipl\I18n\Locale; use ipl\I18n\StaticTranslator; @@ -185,7 +186,15 @@ class PreferenceForm extends Form false ); } - + $config = Config::app()->getSection('global'); + if (!isset($config->config_resource)) { + $missingConfigResource = + Text::create($this->translate( + 'No Configuration Database selected.' + . 'To establish a valid database connection set the Configuration Database field.' + )); + $this->warning($missingConfigResource, false); + } $themeFile = StyleSheet::getThemeFile(Config::app()->get('themes', 'default')); if (! (bool) Config::app()->get('themes', 'disabled', false)) { $themes = Icinga::app()->getThemes(); @@ -439,7 +448,7 @@ class PreferenceForm extends Form public function isSubmitted() { - if (parent::isSubmitted()) { + if (($this->getElement('btn_submit') !== null ) && parent::isSubmitted()) { return true; } diff --git a/application/views/scripts/about/index.phtml b/application/views/scripts/about/index.phtml index 31d6863cc..c7d102f7a 100644 --- a/application/views/scripts/about/index.phtml +++ b/application/views/scripts/about/index.phtml @@ -1,8 +1,10 @@ - - hasPendingMigrations(); } catch (Throwable $e) { - throw new LogicException('Please check the database connection in Configuration -> Application -> Resources'); + // suppress } - if ($hasPending): ?> -
-

translate('Pending Migrations') ?>

- + ?> + + + getSection('global') ?> +
+ getPendingMigrations() || (!isset($config->config_resource))) : ?> +

translate('Pending Migrations') ?>

+ +
+ + + + + +
+ config_resource)) : ?> + translate('No Configuration Database selected. To establish a valid database connection set: ')) + ?> + qlink( + $this->translate('Configuration Database'), + 'config/general/', + array('name' => ('Configuration Database')), + array('title' => sprintf($this->translate('Go to Application/General')), 'Configuration Database') + ) ?> +
+ + + getPendingMigrations() as $migration): ?> diff --git a/library/Icinga/Web/StyleSheet.php b/library/Icinga/Web/StyleSheet.php index 65cbb9705..86b1e98c7 100644 --- a/library/Icinga/Web/StyleSheet.php +++ b/library/Icinga/Web/StyleSheet.php @@ -82,6 +82,7 @@ class StyleSheet 'css/icinga/health.less', 'css/icinga/php-diff.less', 'css/icinga/pending-migration.less', + 'css/icinga/db-connection-warning.less', ]; /** diff --git a/public/css/icinga/db-connection-warning.less b/public/css/icinga/db-connection-warning.less new file mode 100644 index 000000000..1ade0f52f --- /dev/null +++ b/public/css/icinga/db-connection-warning.less @@ -0,0 +1,32 @@ +.db-connection-warning { + border-radius: .25em; + display: flex; + align-items: center; + margin: 0 0 1em 0; + padding: 1em 1em; + background-color: fade(#ffaa44, 40%); + color: #fff; + + p { + margin: 0; + align-items: center; + } + + a { + margin: 0; + align-items: center; + margin-left: .5em; + color: #00c3ed; + } + + i { + font-size: 2em; + + opacity: .4; + align-self: flex-start; + } + + ul { + list-style: none; + } +}
escape($migration->getName()) ?>