From 8ac1174ace67c82d00947e7d02dec4745a0cf530 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Fri, 10 Apr 2026 10:56:03 +0200 Subject: [PATCH 1/2] Make sure that 8.2 is forced when upgrading. When you run composer install with php 8.5 it will break. Therefore, we have to lock the version to 8.2 this is bcuz we want to support from 8.2 onwards not from 8.5 --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index 04a3f59..3040a38 100644 --- a/composer.json +++ b/composer.json @@ -45,6 +45,9 @@ ] }, "config": { + "platform": { + "php": "8.2" + }, "preferred-install": { "*": "dist" }, From f1420eab643455409b5594f8cf03c06f84ec93db Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Fri, 10 Apr 2026 13:23:24 +0200 Subject: [PATCH 2/2] 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. --- src/Controller/HealthController.php | 5 +- .../DoctrineConnectionHealthCheck.php | 11 +- .../DoctrineConnectionHealthCheckTest.php | 121 ++++++++++++++++++ 3 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 tests/HealthCheck/DoctrineConnectionHealthCheckTest.php diff --git a/src/Controller/HealthController.php b/src/Controller/HealthController.php index 7dfb78c..162f880 100644 --- a/src/Controller/HealthController.php +++ b/src/Controller/HealthController.php @@ -20,6 +20,7 @@ use OpenConext\MonitorBundle\HealthCheck\HealthCheckChain; use OpenConext\MonitorBundle\Value\HealthReport; +use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\Routing\Attribute\Route; @@ -34,7 +35,8 @@ class HealthController extends AbstractController { public function __construct( - private readonly HealthCheckChain $healthChecker + private readonly HealthCheckChain $healthChecker, + private readonly LoggerInterface $logger, ) { } @@ -45,6 +47,7 @@ public function __invoke(): JsonResponse try { $statusResponse = $this->healthChecker->check(); } catch (Throwable $exception) { + $this->logger->error('An unexpected error occurred during health checking.', ['exception' => $exception]); $statusResponse = HealthReport::buildStatusDown($exception->getMessage()); } return $this->json($statusResponse, $statusResponse->getStatusCode()); diff --git a/src/HealthCheck/DoctrineConnectionHealthCheck.php b/src/HealthCheck/DoctrineConnectionHealthCheck.php index a9d21f2..5ab562f 100644 --- a/src/HealthCheck/DoctrineConnectionHealthCheck.php +++ b/src/HealthCheck/DoctrineConnectionHealthCheck.php @@ -22,6 +22,7 @@ use Doctrine\DBAL\Exception\ConnectionException; use Exception; use OpenConext\MonitorBundle\Value\HealthReport; +use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\Attribute\Autowire; /** @@ -36,8 +37,8 @@ class DoctrineConnectionHealthCheck implements HealthCheckInterface public function __construct( #[Autowire(service: 'doctrine.dbal.default_connection')] private readonly ?Connection $connection, - ) - { + private readonly LoggerInterface $logger, + ) { } public function check(HealthReportInterface $report): HealthReportInterface @@ -66,9 +67,11 @@ public function check(HealthReportInterface $report): HealthReportInterface $query = "SELECT * FROM %s LIMIT 1"; $this->connection->executeQuery(sprintf($query, $table->getName())); - } catch (ConnectionException) { + } catch (ConnectionException $exception) { + $this->logger->error('Unable to connect to the database.', ['exception' => $exception]); return HealthReport::buildStatusDown('Unable to connect to the database.'); - } catch (Exception) { + } catch (Exception $exception) { + $this->logger->error('Unable to execute a query on the database.', ['exception' => $exception]); return HealthReport::buildStatusDown('Unable to execute a query on the database.'); } diff --git a/tests/HealthCheck/DoctrineConnectionHealthCheckTest.php b/tests/HealthCheck/DoctrineConnectionHealthCheckTest.php new file mode 100644 index 0000000..b1099ab --- /dev/null +++ b/tests/HealthCheck/DoctrineConnectionHealthCheckTest.php @@ -0,0 +1,121 @@ +shouldNotReceive('error'); + + $check = new DoctrineConnectionHealthCheck(null, $logger); + $report = HealthReport::buildStatusUp(); + $result = $check->check($report); + + $this->assertFalse($result->isDown()); + } + + public function testLogsErrorOnConnectionException(): void + { + $exception = m::mock(ConnectionException::class); + + $connection = m::mock(Connection::class); + $connection->shouldAllowMockingProtectedMethods(); + // Note: Connection::connect() is protected in Doctrine DBAL 4 and cannot be made to throw directly. + // The exception is thrown from createSchemaManager() to exercise the ConnectionException catch block. + $connection->shouldReceive('connect'); + $connection->shouldReceive('createSchemaManager')->andThrow($exception); + + $logger = m::mock(LoggerInterface::class); + $logger->shouldReceive('error') + ->once() + ->with('Unable to connect to the database.', ['exception' => $exception]); + + $check = new DoctrineConnectionHealthCheck($connection, $logger); + $result = $check->check(HealthReport::buildStatusUp()); + + $this->assertTrue($result->isDown()); + $this->assertEquals('Unable to connect to the database.', $result->jsonSerialize()['message']); + } + + public function testLogsErrorOnGenericException(): void + { + $exception = new Exception('Query failed'); + + $connection = m::mock(Connection::class); + $connection->shouldAllowMockingProtectedMethods(); + // Note: Connection::connect() is protected in Doctrine DBAL 4 and cannot be made to throw directly. + // The exception is thrown from createSchemaManager() to exercise the generic Exception catch block. + $connection->shouldReceive('connect'); + $connection->shouldReceive('createSchemaManager')->andThrow($exception); + + $logger = m::mock(LoggerInterface::class); + $logger->shouldReceive('error') + ->once() + ->with('Unable to execute a query on the database.', ['exception' => $exception]); + + $check = new DoctrineConnectionHealthCheck($connection, $logger); + $result = $check->check(HealthReport::buildStatusUp()); + + $this->assertTrue($result->isDown()); + $this->assertEquals('Unable to execute a query on the database.', $result->jsonSerialize()['message']); + } + + public function testReturnsUpReportWhenDatabaseIsHealthy(): void + { + $table = m::mock(Table::class); + $table->shouldReceive('getName')->andReturn('some_table'); + + $schemaManager = m::mock(AbstractSchemaManager::class); + $schemaManager->shouldReceive('listTables')->andReturn([$table]); + + $connection = m::mock(Connection::class); + $connection->shouldAllowMockingProtectedMethods(); + $connection->shouldReceive('connect'); + $connection->shouldReceive('createSchemaManager')->andReturn($schemaManager); + $connection->shouldReceive('executeQuery'); + + $logger = m::mock(LoggerInterface::class); + $logger->shouldNotReceive('error'); + + $report = HealthReport::buildStatusUp(); + $check = new DoctrineConnectionHealthCheck($connection, $logger); + $result = $check->check($report); + + $this->assertSame($report, $result); + } +}