Skip to content

Generic exception message to catch-all exception #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arhimede merged 2 commits into from
Mar 24, 2026
Merged

Generic exception message to catch-all exception #97

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3ed1be
Generic exception message to catch-all exception
pinclau Mar 16, 2026
9ff127a
extended AuthorizationMiddlewareFactory for logger interface
pinclau Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/AuthorizationMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface;

use function is_callable;

Expand Down Expand Up @@ -41,10 +42,12 @@ class AuthorizationMiddleware implements MiddlewareInterface
/** @var ResponseFactoryInterface */
protected $responseFactory;

protected ?LoggerInterface $logger;

/**
* @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory
*/
public function __construct(AuthorizationServer $server, $responseFactory)
public function __construct(AuthorizationServer $server, $responseFactory, ?LoggerInterface $logger = null)
Copy link
Copy Markdown
Member

@froschdesign froschdesign Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have here a typical problem: nobody knows about it.
There is a simple rule: what is not documented does not exist.

Copy link
Copy Markdown
Member

@froschdesign froschdesign Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The related factory should also be extended:

mezzio-authentication-oauth2/src/AuthorizationMiddlewareFactory.php

Lines 16 to 20 in 2733dae

return new AuthorizationMiddleware(
$container->get(AuthorizationServer::class),
$this->detectResponseFactory($container)
);
}

The Psr\Container\ContainerInterface defines a has method which can be used here.

Copy link
Copy Markdown
Contributor Author

@pinclau pinclau Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@froschdesign i made changes to the related factory
What should we do about the documentation?

{
$this->server = $server;
if (is_callable($responseFactory)) {
Expand All @@ -54,6 +57,7 @@ public function __construct(AuthorizationServer $server, $responseFactory)
}

$this->responseFactory = $responseFactory;
$this->logger = $logger;
}

/**
Expand All @@ -75,8 +79,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
// for example when the client id is invalid
return $exception->generateHttpResponse($response);
} catch (BaseException $exception) {
$this->logger?->error('Authorization request error', ['exception' => $exception]);
$response = $this->responseFactory->createResponse();
return (new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500))
return (new OAuthServerException('An internal error occurred', 0, 'unknown_error', 500))
->generateHttpResponse($response);
}
}
Expand Down
42 changes: 40 additions & 2 deletions test/AuthorizationMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Psr\Http\Message\StreamInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface;
use RuntimeException;

final class AuthorizationMiddlewareTest extends TestCase
Expand Down Expand Up @@ -131,7 +132,7 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse
$body = $this->createMock(StreamInterface::class);
$body->expects(self::once())
->method('write')
->with(self::stringContains('oauth2 server error'));
->with(self::stringContains('An internal error occurred'));

$this->response
->expects(self::once())
Expand All @@ -150,7 +151,7 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse
[500, '', $this->response],
]);

$exception = new RuntimeException('oauth2 server error');
$exception = new RuntimeException('An internal error occurred');

$this->authServer->expects(self::once())
->method('validateAuthorizationRequest')
Expand All @@ -169,4 +170,41 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse

self::assertSame($this->response, $response);
}

public function testAuthorizationRequestRaisingUnknownExceptionLogsError(): void
{
$exception = new RuntimeException('An internal error occurred');

$this->authServer->expects(self::once())
->method('validateAuthorizationRequest')
->with($this->serverRequest)
->willThrowException($exception);

$logger = $this->createMock(LoggerInterface::class);
$logger->expects(self::once())
->method('error')
->with(
'Authorization request error',
['exception' => $exception]
);

$body = $this->createMock(StreamInterface::class);
$body->method('write');
$this->response->method('getBody')->willReturn($body);
$this->response->method('withHeader')->willReturnSelf();
$this->response->method('withStatus')->willReturnSelf();

$middleware = new AuthorizationMiddleware(
$this->authServer,
$this->responseFactory,
$logger
);

$response = $middleware->process(
$this->serverRequest,
$this->handler
);

self::assertSame($this->response, $response);
}
}