Skip to content

Commit 22ea6f9

Browse files
turegjorupclaude
andcommitted
feat(logging): PHPStan logging gate + silent-catch migration
Add three project-local PHPStan rules (ADR 011 / docs/logging.md) and resolve every site they flag in the same change — zero baseline entries: - NoSilentCatchRule (logging.silentCatch): flags a catch that neither rethrows, logs via PSR-3, calls error_log/trigger_error, nor reports the failure to the console (SymfonyStyle/OutputInterface). The console-output exemption is in the rule, not a src/Command/ path exclusion, so a genuinely empty catch in a command (unattended cron/Messenger) is still flagged. - NoInterpolatedLogMessageRule (logging.interpolatedLogMessage): log message must be a static string; variable data goes in the context array. - ExceptionInContextMustUseExceptionKeyRule (logging.exceptionContextKey): a Throwable in the context array must use the "exception" key. Silent-catch migration: inject channel-bound loggers and log the swallowed exception (AuthOidc/AuthScreenBind, ColiboFeedType, InstantBook, FeedService, Feed/Theme/UserProvider), convert interpolated feed-type messages to static message + exception context, and annotate the genuinely-intentional silences (optional OIDC end-session URL, per-booking time parse, read-degradation in FeedSourceProvider, the log-enrichment guard, validator violation, fixtures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a5a2df5 commit 22ea6f9

31 files changed

Lines changed: 727 additions & 24 deletions

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ All notable changes to this project will be documented in this file.
2929
The HTTP client now logs completed requests at `info` and failures at `error` on the
3030
`outbound_http` channel, thresholded by the new `LOG_LEVEL_OUTBOUND_HTTP` like every
3131
other channel; the `HTTP_CLIENT_LOG_LEVEL` env var is removed (use `LOG_LEVEL_OUTBOUND_HTTP`).
32+
- Enforced the logging conventions in CI with three project-local PHPStan rules
33+
(`logging.silentCatch`, `logging.interpolatedLogMessage`, `logging.exceptionContextKey`) and
34+
migrated the previously silent catch sites to log the exception, surface it, or be explicitly
35+
annotated as intentional.
3236
- Removed the deprecated feed types `SparkleIOFeedType`, `EventDatabaseApiFeedType` and `KobaFeedType`.
3337
Made the unknown-feed-type handling consistent: **reads degrade, writes are rejected.** Feed sources
3438
(and feeds) that reference a removed type keep loading — item and collection reads return them with no

config/services.yaml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,27 @@ services:
8787
arguments:
8888
$logger: '@monolog.logger.auth'
8989

90+
# Channel-bound loggers for classes that log failures (ADR 011 / docs/logging.md).
91+
App\Controller\Api\AuthOidcController:
92+
arguments:
93+
$logger: '@monolog.logger.auth'
94+
95+
App\Controller\Api\AuthScreenBindController:
96+
arguments:
97+
$logger: '@monolog.logger.screen'
98+
99+
App\Feed\ColiboFeedType:
100+
arguments:
101+
$logger: '@monolog.logger.feed'
102+
103+
App\Feed\NotifiedFeedType:
104+
arguments:
105+
$logger: '@monolog.logger.feed'
106+
107+
App\Feed\RssFeedType:
108+
arguments:
109+
$logger: '@monolog.logger.feed'
110+
90111
Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface: '@Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Authentication\AuthenticationFailureHandler'
91112
Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface: '@Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Authentication\AuthenticationSuccessHandler'
92113

@@ -137,6 +158,7 @@ services:
137158
App\InteractiveSlide\InstantBook:
138159
arguments:
139160
$busyIntervalsSource: '%env(string:INSTANT_BOOK_BUSY_INTERVALS_SOURCE)%'
161+
$logger: '@monolog.logger.interactive'
140162

141163
App\Service\KeyVaultService:
142164
arguments:
@@ -182,7 +204,8 @@ services:
182204

183205
App\Service\FeedService:
184206
arguments:
185-
- !tagged_iterator app.feed.feed_type
207+
$feedTypes: !tagged_iterator app.feed.feed_type
208+
$logger: '@monolog.logger.feed'
186209

187210
App\Service\InteractiveSlideService:
188211
arguments:
@@ -308,6 +331,7 @@ services:
308331
tags: [{ name: 'api_platform.state_provider', priority: 2 }]
309332
arguments:
310333
$itemExtensions: !tagged_iterator api_platform.doctrine.orm.query_extension.item
334+
$logger: '@monolog.logger.feed'
311335

312336
# https://api-platform.com/docs/v2.7/core/state-providers/
313337
App\State\MediaProvider:

phpstan.dist.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,7 @@ rules:
1313
# Forbids `$this->addSql(...)` in any file under `migrations/`.
1414
# Future migrations must use Doctrine's Schema tool API for portability.
1515
- App\PhpStan\NoAddSqlInMigrationRule
16+
# Structured-logging conventions (ADR 011 / docs/logging.md).
17+
- App\PhpStan\NoSilentCatchRule
18+
- App\PhpStan\NoInterpolatedLogMessageRule
19+
- App\PhpStan\ExceptionInContextMustUseExceptionKeyRule

src/Controller/Api/AuthOidcController.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use ItkDev\OpenIdConnectBundle\Security\OpenIdConfigurationProviderManager;
1111
use Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Authentication\AuthenticationFailureHandler;
1212
use Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Authentication\AuthenticationSuccessHandler;
13+
use Psr\Log\LoggerInterface;
1314
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
1415
use Symfony\Component\HttpFoundation\JsonResponse;
1516
use Symfony\Component\HttpFoundation\Request;
@@ -28,6 +29,7 @@ public function __construct(
2829
private readonly AzureOidcAuthenticator $oidcAuthenticator,
2930
private readonly AuthenticationSuccessHandler $successHandler,
3031
private readonly AuthenticationFailureHandler $failureHandler,
32+
private readonly LoggerInterface $logger,
3133
) {}
3234

3335
#[Route('/v2/authentication/oidc/token', name: 'authentication_oidc_token', methods: ['GET'])]
@@ -39,6 +41,8 @@ public function getToken(Request $request): Response
3941

4042
return $this->successHandler->handleAuthenticationSuccess($passport->getUser());
4143
} catch (CustomUserMessageAuthenticationException|InvalidProviderException $e) {
44+
$this->logger->warning('OIDC token authentication failed', ['exception' => $e]);
45+
4246
$e = new AuthenticationException($e->getMessage());
4347

4448
return $this->failureHandler->onAuthenticationFailure($request, $e);
@@ -75,7 +79,7 @@ public function getUrls(Request $request): Response
7579
// We allow end session endpoint to not be set.
7680
try {
7781
$endSessionUrl = $provider->getEndSessionUrl();
78-
} catch (OpenIdConnectExceptionInterface) {
82+
} catch (OpenIdConnectExceptionInterface) { // @phpstan-ignore logging.silentCatch (the end-session endpoint is optional; its absence is an expected configuration, not a failure)
7983
$endSessionUrl = null;
8084
}
8185

src/Controller/Api/AuthScreenBindController.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use App\Security\ScreenAuthenticator;
1010
use App\Utils\ValidationUtils;
1111
use Psr\Cache\InvalidArgumentException;
12+
use Psr\Log\LoggerInterface;
1213
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
1314
use Symfony\Component\HttpFoundation\JsonResponse;
1415
use Symfony\Component\HttpFoundation\Request;
@@ -22,6 +23,7 @@ public function __construct(
2223
private readonly ScreenAuthenticator $authScreenService,
2324
private readonly ValidationUtils $validationUtils,
2425
private readonly ScreenRepository $screenRepository,
26+
private readonly LoggerInterface $logger,
2527
) {}
2628

2729
public function __invoke(Request $request, string $id): Response
@@ -42,7 +44,9 @@ public function __invoke(Request $request, string $id): Response
4244

4345
try {
4446
$this->authScreenService->bindScreen($screen, $bindKey);
45-
} catch (\Exception|InvalidArgumentException) {
47+
} catch (\Exception|InvalidArgumentException $e) {
48+
$this->logger->error('Screen bind failed', ['exception' => $e, 'screen_id' => (string) $screenUlid]);
49+
4650
return new JsonResponse('Key not accepted', Response::HTTP_BAD_REQUEST);
4751
}
4852

src/DataFixtures/Faker/Provider/RRuleProvider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public static function rrule(): ?RRuleInterface
3636
'count' => $faker->numberBetween(1, 20),
3737
'dtstart' => $faker->dateTimeBetween('now', '+10 weeks'),
3838
]);
39-
} catch (\Exception $exception) {
39+
} catch (\Exception $exception) { // @phpstan-ignore logging.silentCatch (dev-only fixture provider; exit() prints the message and aborts the fixtures load)
4040
exit($exception->getMessage());
4141
}
4242

src/Feed/BrndFeedType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ private function parseBrndBooking(array $booking): array
166166
if (false === $startDateTime) {
167167
$startDateTime = null;
168168
}
169-
} catch (\ValueError) {
169+
} catch (\ValueError) { // @phpstan-ignore logging.silentCatch (a malformed/unparseable start time is an expected per-booking data condition; null is a valid value for the optional field)
170170
$startDateTime = null;
171171
}
172172
}
@@ -182,7 +182,7 @@ private function parseBrndBooking(array $booking): array
182182
if (false === $endDateTime) {
183183
$endDateTime = null;
184184
}
185-
} catch (\ValueError) {
185+
} catch (\ValueError) { // @phpstan-ignore logging.silentCatch (a malformed/unparseable end time is an expected per-booking data condition; null is a valid value for the optional field)
186186
$endDateTime = null;
187187
}
188188
}

src/Feed/ColiboFeedType.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use FeedIo\Feed\Node\Category;
1515
use Psr\Cache\CacheItemInterface;
1616
use Psr\Cache\CacheItemPoolInterface;
17+
use Psr\Log\LoggerInterface;
1718
use Symfony\Component\DomCrawler\Crawler;
1819
use Symfony\Component\HttpFoundation\Request;
1920
use Symfony\Component\Uid\Ulid;
@@ -34,6 +35,7 @@ public function __construct(
3435
private readonly FeedService $feedService,
3536
private readonly ApiClient $apiClient,
3637
private readonly CacheItemPoolInterface $feedsCache,
38+
private readonly LoggerInterface $logger,
3739
) {}
3840

3941
public function getAdminFormOptions(FeedSource $feedSource): array
@@ -133,7 +135,8 @@ public function getData(Feed $feed): array
133135
if (null !== $entry->fields->galleryItems) {
134136
try {
135137
$galleryItems = json_decode($entry->fields->galleryItems, true, 512, JSON_THROW_ON_ERROR);
136-
} catch (\JsonException) {
138+
} catch (\JsonException $e) {
139+
$this->logger->warning('Malformed gallery items in Colibo feed entry', ['exception' => $e]);
137140
$galleryItems = [];
138141
}
139142

src/Feed/NotifiedFeedType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function getData(Feed $feed): array
5959
try {
6060
$mentions = $this->getMentions($token, 1, $pageSize, $configuration['feeds']);
6161
} catch (\Throwable $throwable) {
62-
$this->logger->error("NotifiedFeedType: Failed to get mentions: {$throwable->getMessage()}");
62+
$this->logger->error('Failed to get mentions', ['exception' => $throwable]);
6363
}
6464

6565
$result = [];
@@ -77,7 +77,7 @@ public function getData(Feed $feed): array
7777
try {
7878
$response = $this->client->request(Request::METHOD_HEAD, $mediaUrl);
7979
} catch (\Throwable $throwable) {
80-
$this->logger->error("NotifiedFeedType: Failed to get mediaUrl: {$throwable->getMessage()}");
80+
$this->logger->error('Failed to fetch media URL', ['exception' => $throwable, 'media_url' => $mediaUrl]);
8181
continue;
8282
}
8383

src/Feed/RssFeedType.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function getData(Feed $feed): array
7373

7474
return $result;
7575
} catch (\Throwable $throwable) {
76-
$this->logger->error($throwable->getCode().': '.$throwable->getMessage());
76+
$this->logger->error('RSS feed processing failed', ['exception' => $throwable]);
7777

7878
throw $throwable;
7979
}

0 commit comments

Comments
 (0)