Skip to content

Commit 6049208

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 556958a commit 6049208

23 files changed

Lines changed: 469 additions & 24 deletions

config/services.yaml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,27 @@ services:
8080
arguments:
8181
$logger: '@monolog.logger.auth'
8282

83+
# Channel-bound loggers for classes that log failures (ADR 011 / docs/logging.md).
84+
App\Controller\Api\AuthOidcController:
85+
arguments:
86+
$logger: '@monolog.logger.auth'
87+
88+
App\Controller\Api\AuthScreenBindController:
89+
arguments:
90+
$logger: '@monolog.logger.screen'
91+
92+
App\Feed\ColiboFeedType:
93+
arguments:
94+
$logger: '@monolog.logger.feed'
95+
96+
App\Feed\NotifiedFeedType:
97+
arguments:
98+
$logger: '@monolog.logger.feed'
99+
100+
App\Feed\RssFeedType:
101+
arguments:
102+
$logger: '@monolog.logger.feed'
103+
83104
Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface: '@Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Authentication\AuthenticationFailureHandler'
84105
Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface: '@Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Authentication\AuthenticationSuccessHandler'
85106

@@ -130,6 +151,7 @@ services:
130151
App\InteractiveSlide\InstantBook:
131152
arguments:
132153
$busyIntervalsSource: '%env(string:INSTANT_BOOK_BUSY_INTERVALS_SOURCE)%'
154+
$logger: '@monolog.logger.interactive'
133155

134156
App\Service\KeyVaultService:
135157
arguments:
@@ -175,7 +197,8 @@ services:
175197

176198
App\Service\FeedService:
177199
arguments:
178-
- !tagged_iterator app.feed.feed_type
200+
$feedTypes: !tagged_iterator app.feed.feed_type
201+
$logger: '@monolog.logger.feed'
179202

180203
App\Service\InteractiveSlideService:
181204
arguments:
@@ -301,6 +324,7 @@ services:
301324
tags: [{ name: 'api_platform.state_provider', priority: 2 }]
302325
arguments:
303326
$itemExtensions: !tagged_iterator api_platform.doctrine.orm.query_extension.item
327+
$logger: '@monolog.logger.feed'
304328

305329
# https://api-platform.com/docs/v2.7/core/state-providers/
306330
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 (ItkOpenIdConnectException) {
82+
} catch (ItkOpenIdConnectException) { // @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
}

src/InteractiveSlide/InstantBook.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use App\Service\KeyVaultService;
2020
use Psr\Cache\CacheItemInterface;
2121
use Psr\Cache\InvalidArgumentException;
22+
use Psr\Log\LoggerInterface;
2223
use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException;
2324
use Symfony\Contracts\Cache\CacheInterface;
2425
use Symfony\Contracts\HttpClient\HttpClientInterface;
@@ -62,6 +63,7 @@ public function __construct(
6263
private readonly CacheInterface $interactiveSlideCache,
6364
private readonly FeedService $feedService,
6465
private readonly string $busyIntervalsSource,
66+
private readonly LoggerInterface $logger,
6567
) {
6668
if (!in_array($busyIntervalsSource, [self::SOURCE_GRAPH, self::SOURCE_FEED], true)) {
6769
throw new \InvalidArgumentException(sprintf('Invalid INSTANT_BOOK_BUSY_INTERVALS_SOURCE "%s"; expected "%s" or "%s".', $busyIntervalsSource, self::SOURCE_GRAPH, self::SOURCE_FEED));
@@ -231,8 +233,10 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest,
231233
$updatedWatchedResources = $watchedResources;
232234

233235
return $result;
234-
} catch (\Throwable) {
236+
} catch (\Throwable $e) {
235237
// All errors should result in empty options.
238+
$this->logger->error('Failed to compute instant-book options; returning empty options', ['exception' => $e]);
239+
236240
return $this->createEntry($resource, $start);
237241
}
238242
}
@@ -268,7 +272,8 @@ private function createEntry(string $resource, \DateTime $start, ?array $schedul
268272
foreach (self::DURATIONS as $durationMinutes) {
269273
try {
270274
$startPlus = (clone $start)->add(new \DateInterval('PT'.$durationMinutes.'M'))->setTimezone(new \DateTimeZone('UTC'));
271-
} catch (\Exception) {
275+
} catch (\Exception $e) {
276+
$this->logger->error('Failed to build instant-book duration interval; skipping duration', ['exception' => $e, 'duration_minutes' => $durationMinutes]);
272277
continue;
273278
}
274279

0 commit comments

Comments
 (0)