Skip to content

Commit 15b0148

Browse files
JolienTrogJolienTrog
committed
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
1 parent cca7f2d commit 15b0148

File tree

8 files changed

+190
-38
lines changed

8 files changed

+190
-38
lines changed

‎application/controllers/ErrorController.php

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Icinga\Application\Hook\DbMigrationHook;
77
use Icinga\Application\MigrationManager;
88
use Icinga\Exception\IcingaException;
9+
use Throwable;
910
use Zend_Controller_Plugin_ErrorHandler;
1011
use Icinga\Application\Icinga;
1112
use Icinga\Application\Logger;
@@ -96,17 +97,23 @@ public function errorAction()
9697
$mm = MigrationManager::instance();
9798
$action = $this->getRequest()->getActionName();
9899
$controller = $this->getRequest()->getControllerName();
99-
if ($action !== 'hint' && $controller !== 'migrations' && $mm->hasMigrations($moduleName)) {
100-
// The view renderer from IPL web doesn't render the HTML content set in the respective
101-
// controller if the error_handler request param is set, as it doesn't support error
102-
// rendering. Since this error handler isn't caused by the migrations controller, we can
103-
// safely unset this.
104-
$this->setParam('error_handler', null);
105-
$this->forward('hint', 'migrations', 'default', [
106-
DbMigrationHook::MIGRATION_PARAM => $moduleName
107-
]);
108-
109-
return;
100+
$hasPending = false;
101+
try {
102+
$hasPending = $mm->hasPendingMigrations();
103+
if ($action !== 'hint' && $controller !== 'migrations' && $mm->hasMigrations($moduleName)) {
104+
// The view renderer from IPL web doesn't render the HTML content set in the respective
105+
// controller if the error_handler request param is set, as it doesn't support error
106+
// rendering. Since this error handler isn't caused by the migrations controller, we can
107+
// safely unset this.
108+
$this->setParam('error_handler', null);
109+
$this->forward('hint', 'migrations', 'default', [
110+
DbMigrationHook::MIGRATION_PARAM => $moduleName
111+
]);
112+
113+
return;
114+
}
115+
} catch (Throwable $e) {
116+
//suppress
110117
}
111118

112119
$this->getResponse()->setHttpResponseCode(500);

‎application/controllers/MigrationsController.php

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
use ipl\Html\Text;
2020
use ipl\Web\Compat\CompatController;
2121
use ipl\Web\Widget\ActionLink;
22+
use ipl\Web\Widget\Icon;
23+
use ipl\Web\Widget\Link;
24+
use Throwable;
2225

2326
class MigrationsController extends CompatController
2427
{
@@ -55,24 +58,48 @@ public function indexAction(): void
5558

5659
$migrateListForm = new MigrationForm();
5760
$migrateListForm->setAttribute('id', $this->getRequest()->protectId('migration-form'));
58-
$migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges());
61+
try {
62+
$migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges());
5963

60-
if ($canApply && $mm->hasPendingMigrations()) {
61-
$migrateAllButton = new SubmitButtonElement(sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [
64+
if ($canApply && $mm->hasPendingMigrations()) {
65+
$migrateAllButton = new SubmitButtonElement(sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [
6266
'form' => $migrateListForm->getAttribute('id')->getValue(),
6367
'label' => $this->translate('Migrate All'),
6468
'title' => $this->translate('Migrate all pending migrations')
65-
]);
69+
]);
6670

67-
// Is the first button, so will be cloned and that the visible
68-
// button is outside the form doesn't matter for Web's JS
69-
$migrateListForm->registerElement($migrateAllButton);
71+
// Is the first button, so will be cloned and that the visible
72+
// button is outside the form doesn't matter for Web's JS
73+
$migrateListForm->registerElement($migrateAllButton);
7074

71-
// Make sure it looks familiar, even if not inside a form
72-
$migrateAllButton->setWrapper(new HtmlElement('div', Attributes::create(['class' => 'icinga-controls'])));
75+
// Make sure it looks familiar, even if not inside a form
76+
$migrateAllButton->setWrapper(
77+
new HtmlElement('div', Attributes::create(['class' => 'icinga-controls']))
78+
);
7379

74-
$this->controls->getAttributes()->add('class', 'default-layout');
75-
$this->addControl($migrateAllButton);
80+
$this->controls->getAttributes()->add('class', 'default-layout');
81+
$this->addControl($migrateAllButton);
82+
}
83+
} catch (Throwable $e) {
84+
$this->addContent(
85+
new HtmlElement(
86+
'div',
87+
new Attributes(['class' => 'db-connection-warning']),
88+
new Icon('warning'),
89+
new HtmlElement('ul', null),
90+
new HtmlElement(
91+
'p',
92+
null,
93+
new Text(
94+
$this->translate(' No Configuration Database selected.
95+
To establish a valid database connection set the Configuration Database field. ')
96+
)
97+
),
98+
new HtmlElement('ul', null),
99+
new Link($this->translate('Configuration Database'), 'config/general/')
100+
)
101+
);
102+
return;
76103
}
77104

78105
$this->handleFormatRequest($mm->toArray());

‎application/controllers/MyDevicesController.php

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,15 @@
44
namespace Icinga\Controllers;
55

66
use Icinga\Common\Database;
7-
use Icinga\Web\Notification;
87
use Icinga\Web\RememberMe;
98
use Icinga\Web\RememberMeUserDevicesList;
9+
use ipl\Html\Attributes;
10+
use ipl\Html\HtmlElement;
11+
use ipl\Html\Text;
1012
use ipl\Web\Compat\CompatController;
13+
use ipl\Web\Widget\Icon;
14+
use ipl\Web\Widget\Link;
15+
use Throwable;
1116

1217
/**
1318
* MyDevicesController
@@ -49,6 +54,46 @@ public function init()
4954

5055
public function indexAction()
5156
{
57+
try {
58+
$this->getDb();
59+
} catch (Throwable $e) {
60+
$hasConfigPermission = $this->hasPermission('config/*');
61+
if ($hasConfigPermission) {
62+
$this->addContent(
63+
new HtmlElement(
64+
'div',
65+
new Attributes(['class' => 'db-connection-warning']),
66+
new Icon('warning'),
67+
new HtmlElement(
68+
'p',
69+
null,
70+
new Text($this->translate(
71+
'No Configuration Database selected.'
72+
. 'To establish a valid database connection set the Configuration Database field.'
73+
))
74+
),
75+
new Link($this->translate('Configuration Database'), 'config/general/')
76+
)
77+
);
78+
} else {
79+
$this->addContent(
80+
new HtmlElement(
81+
'div',
82+
new Attributes(['class' => 'db-connection-warning']),
83+
new Icon('warning'),
84+
new HtmlElement(
85+
'p',
86+
null,
87+
new Text($this->translate(
88+
'No Configuration Database selected.'
89+
. 'You don`t have permission to change this setting. Please contact an administrator.'
90+
))
91+
)
92+
)
93+
);
94+
}
95+
return;
96+
}
5297
$name = $this->auth->getUser()->getUsername();
5398

5499
$data = (new RememberMeUserDevicesList())
@@ -57,12 +102,6 @@ public function indexAction()
57102
->setUrl('my-devices/delete');
58103

59104
$this->addContent($data);
60-
61-
if (! $this->hasDb()) {
62-
Notification::warning(
63-
$this->translate("Users can't stay logged in without a database configuration backend")
64-
);
65-
}
66105
}
67106

68107
public function deleteAction()

‎application/forms/Config/General/ApplicationConfigForm.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33

44
namespace Icinga\Forms\Config\General;
55

6+
use Icinga\Application\Config;
67
use Icinga\Application\Icinga;
78
use Icinga\Data\ResourceFactory;
89
use Icinga\Web\Form;
10+
use ipl\Html\Text;
911

1012
/**
1113
* Configuration form for general application options
@@ -100,6 +102,16 @@ public function createElements(array $formData)
100102
)
101103
);
102104

105+
$config = Config::app()->getSection('global');
106+
if (!isset($config->config_resource)) {
107+
$missingConfigResource =
108+
Text::create(
109+
$this->translate("No Configuration Database selected.
110+
Please set the field to establish a valid database connection.")
111+
);
112+
$this->warning($missingConfigResource, false);
113+
}
114+
103115
return $this;
104116
}
105117
}

‎application/forms/PreferenceForm.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Icinga\Web\Session;
1818
use Icinga\Web\StyleSheet;
1919
use ipl\Html\HtmlElement;
20+
use ipl\Html\Text;
2021
use ipl\I18n\GettextTranslator;
2122
use ipl\I18n\Locale;
2223
use ipl\I18n\StaticTranslator;
@@ -185,7 +186,15 @@ public function createElements(array $formData)
185186
false
186187
);
187188
}
188-
189+
$config = Config::app()->getSection('global');
190+
if (!isset($config->config_resource)) {
191+
$missingConfigResource =
192+
Text::create($this->translate(
193+
'No Configuration Database selected.'
194+
. 'To establish a valid database connection set the Configuration Database field.'
195+
));
196+
$this->warning($missingConfigResource, false);
197+
}
189198
$themeFile = StyleSheet::getThemeFile(Config::app()->get('themes', 'default'));
190199
if (! (bool) Config::app()->get('themes', 'disabled', false)) {
191200
$themes = Icinga::app()->getThemes();
@@ -439,7 +448,7 @@ public function addSubmitButton()
439448

440449
public function isSubmitted()
441450
{
442-
if (parent::isSubmitted()) {
451+
if (($this->getElement('btn_submit') !== null ) && parent::isSubmitted()) {
443452
return true;
444453
}
445454

‎application/views/scripts/about/index.phtml

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
<?php
22

3+
use Icinga\Application\Config;
34
use Icinga\Application\MigrationManager;
45
use Icinga\Web\Navigation\Renderer\BadgeNavigationItemRenderer;
56
use ipl\Html\HtmlElement;
7+
use ipl\Web\Widget\EmptyState;
68
use ipl\Web\Widget\Icon;
79
use ipl\Web\Widget\StateBadge;
810

@@ -93,21 +95,44 @@ use ipl\Web\Widget\StateBadge;
9395
]
9496
);
9597
?>
96-
</div>
97-
</div>
9898

9999
<?php
100100
$mm = MigrationManager::instance();
101101
$hasPending = false;
102102
try {
103103
$hasPending = $mm->hasPendingMigrations();
104104
} catch (Throwable $e) {
105-
throw new LogicException('Please check the database connection in Configuration -> Application -> Resources');
105+
// suppress
106106
}
107-
if ($hasPending): ?>
108-
<div class="pending-migrations clearfix">
109-
<h2><?= $this->translate('Pending Migrations') ?></h2>
110-
<table class="name-value-table migrations">
107+
?>
108+
</div>
109+
</div>
110+
<?php $config = Config::app()->getSection('global') ?>
111+
<div class="pending-migrations clearfix">
112+
<?php if($mm->getPendingMigrations() || (!isset($config->config_resource))) : ?>
113+
<h2><?= $this->translate('Pending Migrations') ?></h2>
114+
<?php endif ?>
115+
<table >
116+
<tr>
117+
<th>
118+
<?php if (!isset($config->config_resource)) : ?>
119+
<?= new EmptyState(
120+
$this->translate('No Configuration Database selected. To establish a valid database connection set: '))
121+
?><td>
122+
<?= $this->qlink(
123+
$this->translate('Configuration Database'),
124+
'config/general/',
125+
array('name' => ('Configuration Database')),
126+
array('title' => sprintf($this->translate('Go to Application/General')), 'Configuration Database')
127+
) ?>
128+
</td>
129+
<?php endif ?>
130+
</th>
131+
</tr>
132+
</table>
133+
134+
<?php if ($hasPending): ?>
135+
<table class="name-value-table migrations">
111136
<?php foreach ($mm->getPendingMigrations() as $migration): ?>
112137
<tr>
113138
<th><?= $this->escape($migration->getName()) ?></th>

‎library/Icinga/Web/StyleSheet.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ class StyleSheet
8282
'css/icinga/health.less',
8383
'css/icinga/php-diff.less',
8484
'css/icinga/pending-migration.less',
85+
'css/icinga/db-connection-warning.less',
8586
];
8687

8788
/**

‎public/css/icinga/db-connection-warning.less

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
.db-connection-warning {
2+
border-radius: .25em;
3+
display: flex;
4+
align-items: center;
5+
margin: 0 0 1em 0;
6+
padding: 1em 1em;
7+
background-color: fade(#ffaa44, 40%);
8+
color: #fff;
9+
10+
p {
11+
margin: 0;
12+
align-items: center;
13+
}
14+
15+
a {
16+
margin: 0;
17+
align-items: center;
18+
margin-left: .5em;
19+
color: #00c3ed;
20+
}
21+
22+
i {
23+
font-size: 2em;
24+
25+
opacity: .4;
26+
align-self: flex-start;
27+
}
28+
29+
ul {
30+
list-style: none;
31+
}
32+
}

0 commit comments

Comments
 (0)