Skip to content

Commit

Permalink
Update for review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
trowski committed Nov 13, 2024
1 parent a41c043 commit f9f7e41
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/DefaultExceptionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function __construct(
) {
}

public function handleException(\Throwable $exception, Request $request): Response
public function handleException(Request $request, \Throwable $exception): Response
{
$client = $request->getClient();
$method = $request->getMethod();
Expand Down
31 changes: 9 additions & 22 deletions src/Driver/Internal/AbstractHttpDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ final protected static function getTimeoutQueue(): TimeoutQueue
return self::$timeoutQueue ??= new TimeoutQueue();
}

private ?ExceptionHandler $exceptionHandler = null;
private readonly DefaultExceptionHandler $exceptionHandler;

private int $pendingRequestHandlerCount = 0;
private int $pendingResponseCount = 0;
Expand All @@ -37,6 +37,7 @@ protected function __construct(
protected readonly PsrLogger $logger,
) {
$this->errorHandler = new HttpDriverErrorHandler($errorHandler, $this->logger);
$this->exceptionHandler = new DefaultExceptionHandler($this->errorHandler, $this->logger);
}

/**
Expand All @@ -55,9 +56,14 @@ final protected function handleRequest(Request $request): void
} catch (ClientException $exception) {
throw $exception;
} catch (HttpErrorException $exception) {
$response = $this->handleError($exception->getStatus(), $exception->getReason(), $request);
$response = $this->errorHandler->handleError($exception->getStatus(), $exception->getReason(), $request);
} catch (\Throwable $exception) {
$response = $this->handleInternalServerError($request, $exception);
/**
* This catch is not designed to be a general-purpose exception handler, rather a last-resort to write to
* the logger if the application has failed to handle an exception thrown from a {@see RequestHandler}.
* Instead of relying on this handler, use {@see ExceptionHandler} and {@see ExceptionHandlerMiddleware}.
*/
$response = $this->exceptionHandler->handleException($request, $exception);
} finally {
$this->pendingRequestHandlerCount--;
}
Expand All @@ -83,23 +89,4 @@ final protected function handleRequest(Request $request): void
* Write the given response to the client.
*/
abstract protected function write(Request $request, Response $response): void;

/**
* Used if an exception is thrown from a request handler.
*
* This is not designed to be a general-purpose exception handler, rather a last-resort to write to the logger
* if the application has failed to handle an exception thrown from a {@see RequestHandler}. Instead of relying on
* this handler, use {@see ExceptionHandlerMiddleware} and {@see ExceptionHandler}.
*/
private function handleInternalServerError(Request $request, \Throwable $exception): Response
{
$this->exceptionHandler ??= new DefaultExceptionHandler($this->errorHandler, $this->logger);

return $this->exceptionHandler->handleException($exception, $request);
}

private function handleError(int $status, ?string $reason, Request $request): Response
{
return $this->errorHandler->handleError($status, $reason, $request);
}
}
6 changes: 3 additions & 3 deletions src/Driver/Internal/HttpDriverErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
use Amp\Http\Server\ErrorHandler;
use Amp\Http\Server\Request;
use Amp\Http\Server\Response;
use Psr\Log\LoggerInterface;
use Psr\Log\LoggerInterface as PsrLogger;

/** @internal */
final class HttpDriverErrorHandler implements ErrorHandler
{
private static ?ErrorHandler $defaultErrorHandler = null;
private static ?DefaultErrorHandler $defaultErrorHandler = null;

private static function getDefaultErrorHandler(): ErrorHandler
{
Expand All @@ -20,7 +20,7 @@ private static function getDefaultErrorHandler(): ErrorHandler

public function __construct(
private readonly ErrorHandler $errorHandler,
private readonly LoggerInterface $logger,
private readonly PsrLogger $logger,
) {
}

Expand Down
2 changes: 1 addition & 1 deletion src/ExceptionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ interface ExceptionHandler
/**
* Handles an uncaught exception from the {@see RequestHandler} wrapped with {@see ExceptionHandlerMiddleware}.
*/
public function handleException(\Throwable $exception, Request $request): Response;
public function handleException(Request $request, \Throwable $exception): Response;
}
2 changes: 1 addition & 1 deletion src/Middleware/ExceptionHandlerMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function handleRequest(Request $request, RequestHandler $requestHandler):
// handler.
throw $exception;
} catch (\Throwable $exception) {
return $this->exceptionHandler->handleException($exception, $request);
return $this->exceptionHandler->handleException($request, $exception);
}
}
}

0 comments on commit f9f7e41

Please sign in to comment.