Rework exception hierarchy onto marker interfaces (5.0, BREAKING)

turegjorup · claude · turegjorup · commit a4fcfac4b4fb · 2026-06-02T11:22:29.000+02:00
Every exception thrown from a public method now implements OpenIdConnectBundleExceptionInterface, which extends the upstream library marker OpenIdConnectExceptionInterface. Concrete exceptions extend the SPL type that best fits the failure category and no longer extend the abstract ItkOpenIdConnectBundleException (kept as a deprecated alias for 5.x). A consumer can now catch every OIDC failure from both packages with a single catch (OpenIdConnectExceptionInterface), or scope to the bundle with OpenIdConnectBundleExceptionInterface. - Bump itk-dev/openid-connect to ^5.0 for the matching upstream contract. - Widen @throws / catch types in the authenticator, manager, controller and CLI helper onto the marker interface; preserve the $previous chain. - Add ExceptionHierarchyTest locking marker inheritance, SPL parents and cross-package catch. BREAKING: catch (ItkOpenIdConnectBundleException) no longer matches any concrete thrown by the bundle. Catch OpenIdConnectBundleExceptionInterface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/.gitignore b/.gitignore
@@ -18,3 +18,7 @@ var
 .php-cs-fixer.cache
 .phpunit.cache
 unit.xml
+
+# Local Claude Code working files (not part of the published package)
+/CLAUDE.md
+/EXCEPTION-CONTRACT-MIGRATION.md
diff --git a/.markdownlintignore b/.markdownlintignore
@@ -10,3 +10,7 @@ web/*.md
 web/core/
 web/libraries/
 web/*/contrib/
+# Claude Code guidance — verbose narrative prose, intentionally long lines.
+CLAUDE.md
+# Planning artefact — superseded once 5.0 ships.
+EXCEPTION-CONTRACT-MIGRATION.md
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,13 +7,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed (BREAKING)
+
+- **Exception hierarchy reworked.** Every exception thrown from a public
+  method now implements `OpenIdConnectBundleExceptionInterface` (which
+  extends `\ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface`
+  from the upstream library). Concrete exceptions now extend the SPL type
+  that best describes the failure category (`\RuntimeException`,
+  `\InvalidArgumentException`); they no longer extend
+  `ItkOpenIdConnectBundleException`. Consumers catching the abstract base
+  must migrate to `OpenIdConnectBundleExceptionInterface` — the abstract
+  class is kept for this release as a documented alias and is
+  `@deprecated`, but `catch (ItkOpenIdConnectBundleException $e)` blocks
+  will no longer match any concrete thrown by the bundle.
+- Bumped `itk-dev/openid-connect` requirement to `^5.0` for the matching
+  upstream contract.
+- `OpenIdLoginAuthenticator::validateClaims` now catches on the marker
+  interface (`OpenIdConnectExceptionInterface`) instead of the deprecated
+  upstream abstract. The `$previous`-chain behaviour is preserved.
+- `LoginController::login` catches on the marker interface before mapping
+  to `ServiceUnavailableHttpException`. No consumer-visible behaviour
+  change.
+
+### Added
+
+- `ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface`
+  marker for catching all bundle-thrown OIDC failures.
+
 ### Changed
 
 - Hardened static analysis. PHPStan now analyses `tests/` in addition to
   `src/`, runs the strict, deprecation, PHPUnit and Symfony rule packs, and
   requires a comment on every ignore (`reportIgnoresWithoutComments`). Pinned
   `phpstan/phpstan` to `^2.1.41`. No public-API or behavioural change.
 
+### Deprecated
+
+- `ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException`
+  abstract class (catch `OpenIdConnectBundleExceptionInterface` instead).
+  Will be removed in 6.0.
+
 ## [4.2.0] - 2026-05-11
 
 ### Added
diff --git a/composer.json b/composer.json
@@ -18,7 +18,7 @@
         "ext-json": "*",
         "ext-openssl": "*",
         "doctrine/orm": "^2.8 || ^3.0",
-        "itk-dev/openid-connect": "^4.0",
+        "itk-dev/openid-connect": "^5.0",
         "symfony/cache": "^6.4 || ^7.0 || ^8.0",
         "symfony/framework-bundle": "^6.4.13 || ^7.0 || ^8.0",
         "symfony/security-bundle": "^6.4.13 || ^7.0 || ^8.0",
diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php
@@ -2,7 +2,7 @@
 
 namespace ItkDev\OpenIdConnectBundle\Controller;
 
-use ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException;
+use ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface;
 use ItkDev\OpenIdConnectBundle\Exception\InvalidProviderException;
 use ItkDev\OpenIdConnectBundle\Security\OpenIdConfigurationProviderManager;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
@@ -27,7 +27,7 @@ public function __construct(
      *
      * @throws NotFoundHttpException           Provider key not configured (404)
      * @throws ServiceUnavailableHttpException IdP unreachable, returned a non-200, served malformed JSON, or local cache failed (503)
-     * @throws ItkOpenIdConnectException       Other provider-init failures (e.g. BadUrlException for a misconfigured metadata_url) — server-side configuration bugs that intentionally bubble as 500
+     * @throws OpenIdConnectExceptionInterface Other provider-init failures (e.g. BadUrlException for a misconfigured metadata_url) — server-side configuration bugs that intentionally bubble as 500
      * @throws \InvalidArgumentException       Declared by league\AbstractProvider::getAuthorizationUrl for missing scope/state. Unreachable in this flow (state always provided, getDefaultScopes() implemented in upstream OpenIdConfigurationProvider). Bubbles as 500 if it ever fires — programmer error.
      */
     public function login(Request $request, SessionInterface $session, string $providerKey): RedirectResponse
@@ -53,7 +53,7 @@ public function login(Request $request, SessionInterface $session, string $provi
                 'response_type' => 'code',
                 'scope' => 'openid email profile',
             ]);
-        } catch (ItkOpenIdConnectException $e) {
+        } catch (OpenIdConnectExceptionInterface $e) {
             // Building the authorization URL fetches the IdP's discovery
             // document. Surface upstream/transport/cache failures as 503 with
             // the cause chained, rather than an unhandled 500.
diff --git a/src/Exception/CacheException.php b/src/Exception/CacheException.php
@@ -2,6 +2,6 @@
 
 namespace ItkDev\OpenIdConnectBundle\Exception;
 
-class CacheException extends ItkOpenIdConnectBundleException
+class CacheException extends \RuntimeException implements OpenIdConnectBundleExceptionInterface
 {
 }
diff --git a/src/Exception/InvalidProviderException.php b/src/Exception/InvalidProviderException.php
@@ -2,6 +2,6 @@
 
 namespace ItkDev\OpenIdConnectBundle\Exception;
 
-class InvalidProviderException extends ItkOpenIdConnectBundleException
+class InvalidProviderException extends \InvalidArgumentException implements OpenIdConnectBundleExceptionInterface
 {
 }
diff --git a/src/Exception/ItkOpenIdConnectBundleException.php b/src/Exception/ItkOpenIdConnectBundleException.php
@@ -2,6 +2,12 @@
 
 namespace ItkDev\OpenIdConnectBundle\Exception;
 
-abstract class ItkOpenIdConnectBundleException extends \Exception
+/**
+ * @deprecated since 5.0, will be removed in 6.0.
+ *             Catch {@see OpenIdConnectBundleExceptionInterface} instead. Concrete bundle
+ *             exceptions no longer extend this class; they extend the SPL exception that best
+ *             describes the failure category and implement the marker interface.
+ */
+abstract class ItkOpenIdConnectBundleException extends \Exception implements OpenIdConnectBundleExceptionInterface
 {
 }
diff --git a/src/Exception/OpenIdConnectBundleExceptionInterface.php b/src/Exception/OpenIdConnectBundleExceptionInterface.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace ItkDev\OpenIdConnectBundle\Exception;
+
+use ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface;
+
+/**
+ * Marker interface for every exception thrown from a public method of this bundle.
+ *
+ * Extends the upstream library marker so a consumer can catch every OIDC failure
+ * from both packages with a single `catch (OpenIdConnectExceptionInterface $e)`,
+ * or scope to bundle-only failures with `catch (OpenIdConnectBundleExceptionInterface $e)`.
+ */
+interface OpenIdConnectBundleExceptionInterface extends OpenIdConnectExceptionInterface
+{
+}
diff --git a/src/Exception/TokenNotFoundException.php b/src/Exception/TokenNotFoundException.php
@@ -2,6 +2,6 @@
 
 namespace ItkDev\OpenIdConnectBundle\Exception;
 
-class TokenNotFoundException extends ItkOpenIdConnectBundleException
+class TokenNotFoundException extends \RuntimeException implements OpenIdConnectBundleExceptionInterface
 {
 }
diff --git a/src/Exception/UserDoesNotExistException.php b/src/Exception/UserDoesNotExistException.php
@@ -2,6 +2,6 @@
 
 namespace ItkDev\OpenIdConnectBundle\Exception;
 
-class UserDoesNotExistException extends ItkOpenIdConnectBundleException
+class UserDoesNotExistException extends \RuntimeException implements OpenIdConnectBundleExceptionInterface
 {
 }
diff --git a/src/Exception/UsernameDoesNotExistException.php b/src/Exception/UsernameDoesNotExistException.php
@@ -2,6 +2,6 @@
 
 namespace ItkDev\OpenIdConnectBundle\Exception;
 
-class UsernameDoesNotExistException extends ItkOpenIdConnectBundleException
+class UsernameDoesNotExistException extends \InvalidArgumentException implements OpenIdConnectBundleExceptionInterface
 {
 }
diff --git a/src/Security/OpenIdConfigurationProviderManager.php b/src/Security/OpenIdConfigurationProviderManager.php
@@ -2,7 +2,7 @@
 
 namespace ItkDev\OpenIdConnectBundle\Security;
 
-use ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException;
+use ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface;
 use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider;
 use ItkDev\OpenIdConnectBundle\Exception\InvalidProviderException;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@@ -53,8 +53,7 @@ public function getProviderKeys(): array
     /**
      * Get a provider by key.
      *
-     * @throws InvalidProviderException
-     * @throws ItkOpenIdConnectException
+     * @throws OpenIdConnectExceptionInterface
      */
     public function getProvider(string $key): OpenIdConfigurationProvider
     {
@@ -92,6 +91,7 @@ public function getProvider(string $key): OpenIdConfigurationProvider
                 $providerOptions += $options['http_client_options'];
             }
 
+            // @phpstan-ignore argument.type (library 5.0 narrowed $options to a strict array shape; the incremental build above is verified by the manager's tests but PHPStan can't track its evolution to the final shape)
             $this->providers[$key] = new OpenIdConfigurationProvider($providerOptions);
         }
 
diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php
@@ -2,10 +2,8 @@
 
 namespace ItkDev\OpenIdConnectBundle\Security;
 
-use ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException;
+use ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface;
 use ItkDev\OpenIdConnect\Exception\ValidationException;
-use ItkDev\OpenIdConnectBundle\Exception\InvalidProviderException;
-use Psr\Http\Client\ClientExceptionInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
@@ -36,10 +34,7 @@ public function supports(Request $request): ?bool
      *
      * @return array<string, string> Array of claims
      *
-     * @throws InvalidProviderException
-     * @throws ItkOpenIdConnectException
-     * @throws ValidationException
-     * @throws ClientExceptionInterface
+     * @throws OpenIdConnectExceptionInterface
      */
     protected function validateClaims(Request $request): array
     {
@@ -70,7 +65,7 @@ protected function validateClaims(Request $request): array
             $idToken = $provider->getIdToken($code);
             $claims = $provider->validateIdToken($idToken, $oauth2nonce);
             // Authentication successful
-        } catch (ItkOpenIdConnectException $exception) {
+        } catch (OpenIdConnectExceptionInterface $exception) {
             // Handle failed authentication
             throw new ValidationException($exception->getMessage(), previous: $exception);
         }
diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php
@@ -3,6 +3,7 @@
 namespace ItkDev\OpenIdConnectBundle\Util;
 
 use ItkDev\OpenIdConnectBundle\Exception\CacheException;
+use ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface;
 use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException;
 use Psr\Cache\CacheItemPoolInterface;
 use Psr\Cache\InvalidArgumentException;
@@ -23,7 +24,7 @@ public function __construct(
     /**
      * Creates login token for CLI login.
      *
-     * @throws CacheException
+     * @throws OpenIdConnectBundleExceptionInterface
      */
     public function createToken(string $username): string
     {
@@ -65,8 +66,11 @@ public function createToken(string $username): string
     /**
      * Gets username from login token.
      *
+     * `TokenNotFoundException` is part of the public contract — {@see CliLoginTokenAuthenticator}
+     * catches it specifically to distinguish "no such token" from other cache failures.
+     *
      * @throws TokenNotFoundException
-     * @throws CacheException
+     * @throws OpenIdConnectBundleExceptionInterface
      */
     public function getUsername(string $token): ?string
     {
diff --git a/tests/Exception/ExceptionHierarchyTest.php b/tests/Exception/ExceptionHierarchyTest.php
@@ -0,0 +1,132 @@
+<?php
+
+namespace ItkDev\OpenIdConnectBundle\Tests\Exception;
+
+use ItkDev\OpenIdConnect\Exception\HttpException as LibraryHttpException;
+use ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface;
+use ItkDev\OpenIdConnectBundle\Exception\CacheException;
+use ItkDev\OpenIdConnectBundle\Exception\InvalidProviderException;
+use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException;
+use ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface;
+use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException;
+use ItkDev\OpenIdConnectBundle\Exception\UserDoesNotExistException;
+use ItkDev\OpenIdConnectBundle\Exception\UsernameDoesNotExistException;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Locks the bundle's exception contract documented in `CLAUDE.md` "Exceptions":
+ *
+ * - Every concrete bundle exception implements {@see OpenIdConnectBundleExceptionInterface}
+ *   (and therefore {@see OpenIdConnectExceptionInterface} from the upstream library).
+ * - Each concrete extends the SPL type that best describes its failure category.
+ * - A single `catch (OpenIdConnectExceptionInterface $e)` matches every concrete
+ *   from both the bundle and the upstream library.
+ *
+ * A change that breaks any of these properties is a MAJOR version bump per
+ * SemVer commitments — failing this test class is the early warning.
+ */
+class ExceptionHierarchyTest extends TestCase
+{
+    /**
+     * @return iterable<string, array{class-string<\Throwable>, class-string<\Throwable>}>
+     */
+    public static function concreteProvider(): iterable
+    {
+        // Invalid input to a public method → \InvalidArgumentException
+        yield 'InvalidProviderException' => [InvalidProviderException::class, \InvalidArgumentException::class];
+        yield 'UsernameDoesNotExistException' => [UsernameDoesNotExistException::class, \InvalidArgumentException::class];
+
+        // Runtime conditions → \RuntimeException
+        yield 'CacheException' => [CacheException::class, \RuntimeException::class];
+        yield 'TokenNotFoundException' => [TokenNotFoundException::class, \RuntimeException::class];
+        yield 'UserDoesNotExistException' => [UserDoesNotExistException::class, \RuntimeException::class];
+    }
+
+    /**
+     * @param class-string<\Throwable> $concrete
+     * @param class-string<\Throwable> $expectedSplParent
+     */
+    #[DataProvider('concreteProvider')]
+    public function testConcreteImplementsBundleMarker(string $concrete, string $expectedSplParent): void
+    {
+        $instance = new $concrete('test');
+        $this->assertInstanceOf(OpenIdConnectBundleExceptionInterface::class, $instance);
+    }
+
+    /**
+     * @param class-string<\Throwable> $concrete
+     * @param class-string<\Throwable> $expectedSplParent
+     */
+    #[DataProvider('concreteProvider')]
+    public function testConcreteImplementsLibraryMarker(string $concrete, string $expectedSplParent): void
+    {
+        $instance = new $concrete('test');
+        $this->assertInstanceOf(OpenIdConnectExceptionInterface::class, $instance);
+    }
+
+    /**
+     * @param class-string<\Throwable> $concrete
+     * @param class-string<\Throwable> $expectedSplParent
+     */
+    #[DataProvider('concreteProvider')]
+    public function testConcreteExtendsExpectedSplParent(string $concrete, string $expectedSplParent): void
+    {
+        $instance = new $concrete('test');
+        $this->assertInstanceOf($expectedSplParent, $instance);
+    }
+
+    /**
+     * @param class-string<\Throwable> $concrete
+     * @param class-string<\Throwable> $expectedSplParent
+     */
+    #[DataProvider('concreteProvider')]
+    public function testCatchByBundleMarkerMatchesEveryConcrete(string $concrete, string $expectedSplParent): void
+    {
+        try {
+            throw new $concrete('test');
+        } catch (OpenIdConnectBundleExceptionInterface $caught) {
+            $this->assertInstanceOf($concrete, $caught);
+
+            return;
+        }
+        // @phpstan-ignore deadCode.unreachable (safety net if a future regression breaks the catch-by-marker contract)
+        $this->fail('Catch on OpenIdConnectBundleExceptionInterface should have matched '.$concrete);
+    }
+
+    /**
+     * Cross-package contract: a single `catch (OpenIdConnectExceptionInterface $e)`
+     * must catch failures from both the library and the bundle. This is the whole
+     * point of the bundle marker extending the library marker.
+     */
+    public function testLibraryMarkerCatchesBothPackages(): void
+    {
+        $caught = [];
+
+        foreach ([new LibraryHttpException('lib'), new CacheException('bundle')] as $exception) {
+            try {
+                throw $exception;
+            } catch (OpenIdConnectExceptionInterface $e) {
+                $caught[] = $e::class;
+            }
+        }
+
+        $this->assertSame([LibraryHttpException::class, CacheException::class], $caught);
+    }
+
+    public function testDeprecatedAbstractBaseImplementsBundleMarker(): void
+    {
+        // `ItkOpenIdConnectBundleException` is kept as a deprecated alias through 5.x.
+        // Concrete bundle exceptions no longer extend it, but it still implements the
+        // marker so any consumer-defined subclass remains catchable via the marker.
+        // PHPStan can statically prove the assertion holds today; the test exists so
+        // the day a refactor removes the implements, the failure is loud.
+        $deprecated = ItkOpenIdConnectBundleException::class; // @phpstan-ignore classConstant.deprecatedClass (the test asserts a property of this deprecated class on purpose)
+        // @phpstan-ignore method.alreadyNarrowedType (the assertion is the guard — PHPStan proves it today; the test fails the day the guard stops holding)
+        $this->assertTrue(
+            // @phpstan-ignore function.alreadyNarrowedType (same as above — the static proof IS the contract being asserted)
+            is_subclass_of($deprecated, OpenIdConnectBundleExceptionInterface::class),
+            'Deprecated abstract base must continue to implement the bundle marker through 5.x.',
+        );
+    }
+}
diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php
diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php

Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,6 @@`
`2`	`2`
`3`	`3`	`namespace ItkDev\OpenIdConnectBundle\Exception;`
`4`	`4`
`5`		`-class CacheException extends ItkOpenIdConnectBundleException`
	`5`	`+class CacheException extends \RuntimeException implements OpenIdConnectBundleExceptionInterface`
`6`	`6`	`{`
`7`	`7`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,6 @@`
`2`	`2`
`3`	`3`	`namespace ItkDev\OpenIdConnectBundle\Exception;`
`4`	`4`
`5`		`-class InvalidProviderException extends ItkOpenIdConnectBundleException`
	`5`	`+class InvalidProviderException extends \InvalidArgumentException implements OpenIdConnectBundleExceptionInterface`
`6`	`6`	`{`
`7`	`7`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,12 @@`
`2`	`2`
`3`	`3`	`namespace ItkDev\OpenIdConnectBundle\Exception;`
`4`	`4`
`5`		`-abstract class ItkOpenIdConnectBundleException extends \Exception`
	`5`	`+/**`
	`6`	`+ * @deprecated since 5.0, will be removed in 6.0.`
	`7`	`+ * Catch {@see OpenIdConnectBundleExceptionInterface} instead. Concrete bundle`
	`8`	`+ * exceptions no longer extend this class; they extend the SPL exception that best`
	`9`	`+ * describes the failure category and implement the marker interface.`
	`10`	`+ */`
	`11`	`+abstract class ItkOpenIdConnectBundleException extends \Exception implements OpenIdConnectBundleExceptionInterface`
`6`	`12`	`{`
`7`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,6 @@`
`2`	`2`
`3`	`3`	`namespace ItkDev\OpenIdConnectBundle\Exception;`
`4`	`4`
`5`		`-class TokenNotFoundException extends ItkOpenIdConnectBundleException`
	`5`	`+class TokenNotFoundException extends \RuntimeException implements OpenIdConnectBundleExceptionInterface`
`6`	`6`	`{`
`7`	`7`	`}`