Skip to content

Commit f1420ea

Browse files
committed
Log exceptions at ERROR level in health checks
Fixes #27. Exceptions caught in DoctrineConnectionHealthCheck and HealthController were silently swallowed, making database failures impossible to diagnose from logs. Inject LoggerInterface and log the full exception as context so Monolog includes the stack trace.
1 parent 8ac1174 commit f1420ea

3 files changed

Lines changed: 132 additions & 5 deletions

File tree

src/Controller/HealthController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
use OpenConext\MonitorBundle\HealthCheck\HealthCheckChain;
2222
use OpenConext\MonitorBundle\Value\HealthReport;
23+
use Psr\Log\LoggerInterface;
2324
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
2425
use Symfony\Component\HttpFoundation\JsonResponse;
2526
use Symfony\Component\Routing\Attribute\Route;
@@ -34,7 +35,8 @@
3435
class HealthController extends AbstractController
3536
{
3637
public function __construct(
37-
private readonly HealthCheckChain $healthChecker
38+
private readonly HealthCheckChain $healthChecker,
39+
private readonly LoggerInterface $logger,
3840
) {
3941
}
4042

@@ -45,6 +47,7 @@ public function __invoke(): JsonResponse
4547
try {
4648
$statusResponse = $this->healthChecker->check();
4749
} catch (Throwable $exception) {
50+
$this->logger->error('An unexpected error occurred during health checking.', ['exception' => $exception]);
4851
$statusResponse = HealthReport::buildStatusDown($exception->getMessage());
4952
}
5053
return $this->json($statusResponse, $statusResponse->getStatusCode());

src/HealthCheck/DoctrineConnectionHealthCheck.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Doctrine\DBAL\Exception\ConnectionException;
2323
use Exception;
2424
use OpenConext\MonitorBundle\Value\HealthReport;
25+
use Psr\Log\LoggerInterface;
2526
use Symfony\Component\DependencyInjection\Attribute\Autowire;
2627

2728
/**
@@ -36,8 +37,8 @@ class DoctrineConnectionHealthCheck implements HealthCheckInterface
3637
public function __construct(
3738
#[Autowire(service: 'doctrine.dbal.default_connection')]
3839
private readonly ?Connection $connection,
39-
)
40-
{
40+
private readonly LoggerInterface $logger,
41+
) {
4142
}
4243

4344
public function check(HealthReportInterface $report): HealthReportInterface
@@ -66,9 +67,11 @@ public function check(HealthReportInterface $report): HealthReportInterface
6667
$query = "SELECT * FROM %s LIMIT 1";
6768
$this->connection->executeQuery(sprintf($query, $table->getName()));
6869

69-
} catch (ConnectionException) {
70+
} catch (ConnectionException $exception) {
71+
$this->logger->error('Unable to connect to the database.', ['exception' => $exception]);
7072
return HealthReport::buildStatusDown('Unable to connect to the database.');
71-
} catch (Exception) {
73+
} catch (Exception $exception) {
74+
$this->logger->error('Unable to execute a query on the database.', ['exception' => $exception]);
7275
return HealthReport::buildStatusDown('Unable to execute a query on the database.');
7376
}
7477

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
3+
/**
4+
* Copyright 2026 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
namespace OpenConext\MonitorBundle\Tests\HealthCheck;
20+
21+
use Doctrine\DBAL\Connection;
22+
use Doctrine\DBAL\Exception\ConnectionException;
23+
use Doctrine\DBAL\Schema\AbstractSchemaManager;
24+
use Doctrine\DBAL\Schema\Table;
25+
use Exception;
26+
use Mockery as m;
27+
use OpenConext\MonitorBundle\HealthCheck\DoctrineConnectionHealthCheck;
28+
use OpenConext\MonitorBundle\Value\HealthReport;
29+
use PHPUnit\Framework\TestCase;
30+
use Psr\Log\LoggerInterface;
31+
32+
class DoctrineConnectionHealthCheckTest extends TestCase
33+
{
34+
protected function tearDown(): void
35+
{
36+
m::close();
37+
parent::tearDown();
38+
}
39+
40+
public function testSkipsCheckWhenConnectionIsNull(): void
41+
{
42+
$logger = m::mock(LoggerInterface::class);
43+
$logger->shouldNotReceive('error');
44+
45+
$check = new DoctrineConnectionHealthCheck(null, $logger);
46+
$report = HealthReport::buildStatusUp();
47+
$result = $check->check($report);
48+
49+
$this->assertFalse($result->isDown());
50+
}
51+
52+
public function testLogsErrorOnConnectionException(): void
53+
{
54+
$exception = m::mock(ConnectionException::class);
55+
56+
$connection = m::mock(Connection::class);
57+
$connection->shouldAllowMockingProtectedMethods();
58+
// Note: Connection::connect() is protected in Doctrine DBAL 4 and cannot be made to throw directly.
59+
// The exception is thrown from createSchemaManager() to exercise the ConnectionException catch block.
60+
$connection->shouldReceive('connect');
61+
$connection->shouldReceive('createSchemaManager')->andThrow($exception);
62+
63+
$logger = m::mock(LoggerInterface::class);
64+
$logger->shouldReceive('error')
65+
->once()
66+
->with('Unable to connect to the database.', ['exception' => $exception]);
67+
68+
$check = new DoctrineConnectionHealthCheck($connection, $logger);
69+
$result = $check->check(HealthReport::buildStatusUp());
70+
71+
$this->assertTrue($result->isDown());
72+
$this->assertEquals('Unable to connect to the database.', $result->jsonSerialize()['message']);
73+
}
74+
75+
public function testLogsErrorOnGenericException(): void
76+
{
77+
$exception = new Exception('Query failed');
78+
79+
$connection = m::mock(Connection::class);
80+
$connection->shouldAllowMockingProtectedMethods();
81+
// Note: Connection::connect() is protected in Doctrine DBAL 4 and cannot be made to throw directly.
82+
// The exception is thrown from createSchemaManager() to exercise the generic Exception catch block.
83+
$connection->shouldReceive('connect');
84+
$connection->shouldReceive('createSchemaManager')->andThrow($exception);
85+
86+
$logger = m::mock(LoggerInterface::class);
87+
$logger->shouldReceive('error')
88+
->once()
89+
->with('Unable to execute a query on the database.', ['exception' => $exception]);
90+
91+
$check = new DoctrineConnectionHealthCheck($connection, $logger);
92+
$result = $check->check(HealthReport::buildStatusUp());
93+
94+
$this->assertTrue($result->isDown());
95+
$this->assertEquals('Unable to execute a query on the database.', $result->jsonSerialize()['message']);
96+
}
97+
98+
public function testReturnsUpReportWhenDatabaseIsHealthy(): void
99+
{
100+
$table = m::mock(Table::class);
101+
$table->shouldReceive('getName')->andReturn('some_table');
102+
103+
$schemaManager = m::mock(AbstractSchemaManager::class);
104+
$schemaManager->shouldReceive('listTables')->andReturn([$table]);
105+
106+
$connection = m::mock(Connection::class);
107+
$connection->shouldAllowMockingProtectedMethods();
108+
$connection->shouldReceive('connect');
109+
$connection->shouldReceive('createSchemaManager')->andReturn($schemaManager);
110+
$connection->shouldReceive('executeQuery');
111+
112+
$logger = m::mock(LoggerInterface::class);
113+
$logger->shouldNotReceive('error');
114+
115+
$report = HealthReport::buildStatusUp();
116+
$check = new DoctrineConnectionHealthCheck($connection, $logger);
117+
$result = $check->check($report);
118+
119+
$this->assertSame($report, $result);
120+
}
121+
}

0 commit comments

Comments
 (0)