From 87d17d7af4637bb6acbfe57fe0af0a0aa5c9c4c0 Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Mon, 17 Mar 2025 08:31:09 +0100 Subject: [PATCH 1/8] Add exception handling to downloader --- src/Downloader/Downloader.php | 99 ++++++++---- .../DownloaderMiddlewareInterface.php | 3 +- .../DownloaderMiddlewareAdapter.php | 16 +- .../ExceptionMiddlewareInterface.php | 14 ++ src/Downloader/Middleware/FakeMiddleware.php | 33 ++++ src/Events/ExceptionReceived.php | 17 +++ src/Events/ExceptionReceiving.php | 17 +++ src/Http/FakeClient.php | 20 +++ src/Http/FakeGuzzleException.php | 10 ++ tests/Downloader/DownloaderTest.php | 141 ++++++++++++++---- 10 files changed, 311 insertions(+), 59 deletions(-) create mode 100644 src/Downloader/Middleware/ExceptionMiddlewareInterface.php create mode 100644 src/Events/ExceptionReceived.php create mode 100644 src/Events/ExceptionReceiving.php create mode 100644 src/Http/FakeGuzzleException.php diff --git a/src/Downloader/Downloader.php b/src/Downloader/Downloader.php index 940d7ed..b9da613 100644 --- a/src/Downloader/Downloader.php +++ b/src/Downloader/Downloader.php @@ -13,6 +13,9 @@ namespace RoachPHP\Downloader; +use Exception; +use RoachPHP\Events\ExceptionReceived; +use RoachPHP\Events\ExceptionReceiving; use RoachPHP\Events\RequestDropped; use RoachPHP\Events\RequestSending; use RoachPHP\Events\ResponseDropped; @@ -20,6 +23,7 @@ use RoachPHP\Events\ResponseReceiving; use RoachPHP\Http\ClientInterface; use RoachPHP\Http\Request; +use RoachPHP\Http\RequestException; use RoachPHP\Http\Response; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -53,44 +57,48 @@ public function scheduledRequests(): int return \count($this->requests); } - public function prepare(Request $request): void + public function prepare(Request $request, ?callable $onRejected): void { - foreach ($this->middleware as $middleware) { - $request = $middleware->handleRequest($request); + try { + foreach ($this->middleware as $middleware) { + $request = $middleware->handleRequest($request); + + if ($request->wasDropped()) { + $this->eventDispatcher->dispatch( + new RequestDropped($request), + RequestDropped::NAME, + ); + + return; + } + } + + /** + * @psalm-suppress UnnecessaryVarAnnotation + * + * @var RequestSending $event + */ + $event = $this->eventDispatcher->dispatch( + new RequestSending($request), + RequestSending::NAME, + ); - if ($request->wasDropped()) { + if ($event->request->wasDropped()) { $this->eventDispatcher->dispatch( - new RequestDropped($request), + new RequestDropped($event->request), RequestDropped::NAME, ); return; } - } - - /** - * @psalm-suppress UnnecessaryVarAnnotation - * - * @var RequestSending $event - */ - $event = $this->eventDispatcher->dispatch( - new RequestSending($request), - RequestSending::NAME, - ); - if ($event->request->wasDropped()) { - $this->eventDispatcher->dispatch( - new RequestDropped($event->request), - RequestDropped::NAME, - ); - - return; + $this->requests[] = $event->request; + } catch (Exception $exception) { + $this->onExceptionReceived($exception, $request, $onRejected); } - - $this->requests[] = $event->request; } - public function flush(?callable $callback = null): void + public function flush(?callable $onFullFilled = null, ?callable $onRejected = null): void { $requests = $this->requests; @@ -98,7 +106,7 @@ public function flush(?callable $callback = null): void foreach ($requests as $key => $request) { if ($request->getResponse() !== null) { - $this->onResponseReceived($request->getResponse(), $callback); + $this->onResponseReceived($request->getResponse(), $onFullFilled); unset($requests[$key]); } @@ -108,9 +116,19 @@ public function flush(?callable $callback = null): void return; } - $this->client->pool(\array_values($requests), function (Response $response) use ($callback): void { - $this->onResponseReceived($response, $callback); - }); + $this->client->pool( + \array_values($requests), + function (Response $response) use ($onFullFilled): void { + $this->onResponseReceived($response, $onFullFilled); + }, + function (RequestException $requestException) use ($onRejected): void { + $this->onExceptionReceived( + $requestException->getReason(), + $requestException->getRequest(), + $onRejected, + ); + } + ); } private function onResponseReceived(Response $response, ?callable $callback): void @@ -158,4 +176,25 @@ private function onResponseReceived(Response $response, ?callable $callback): vo $callback($response); } } + + private function onExceptionReceived(\Throwable $exception, Request $request, ?callable $callback): void + { + $this->eventDispatcher->dispatch( + new ExceptionReceiving($exception), + ExceptionReceiving::NAME, + ); + + foreach ($this->middleware as $middleware) { + $request = $middleware->handleException($exception, $request); + } + + $this->eventDispatcher->dispatch( + new ExceptionReceived($exception), + ExceptionReceived::NAME, + ); + + if (null !== $callback) { + $callback($exception, $request); + } + } } diff --git a/src/Downloader/DownloaderMiddlewareInterface.php b/src/Downloader/DownloaderMiddlewareInterface.php index 834359a..3e520fa 100644 --- a/src/Downloader/DownloaderMiddlewareInterface.php +++ b/src/Downloader/DownloaderMiddlewareInterface.php @@ -13,9 +13,10 @@ namespace RoachPHP\Downloader; +use RoachPHP\Downloader\Middleware\ExceptionMiddlewareInterface; use RoachPHP\Downloader\Middleware\RequestMiddlewareInterface; use RoachPHP\Downloader\Middleware\ResponseMiddlewareInterface; -interface DownloaderMiddlewareInterface extends RequestMiddlewareInterface, ResponseMiddlewareInterface +interface DownloaderMiddlewareInterface extends RequestMiddlewareInterface, ResponseMiddlewareInterface, ExceptionMiddlewareInterface { } diff --git a/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php b/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php index cb7219a..b86a2e4 100644 --- a/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php +++ b/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php @@ -13,6 +13,7 @@ namespace RoachPHP\Downloader\Middleware; +use Exception; use RoachPHP\Downloader\DownloaderMiddlewareInterface; use RoachPHP\Http\Request; use RoachPHP\Http\Response; @@ -23,12 +24,12 @@ final class DownloaderMiddlewareAdapter implements DownloaderMiddlewareInterface { private function __construct( - private RequestMiddlewareInterface|ResponseMiddlewareInterface $middleware, + private RequestMiddlewareInterface|ResponseMiddlewareInterface|ExceptionMiddlewareInterface $middleware, ) { } public static function fromMiddleware( - RequestMiddlewareInterface|ResponseMiddlewareInterface $middleware, + RequestMiddlewareInterface|ResponseMiddlewareInterface|ExceptionMiddlewareInterface $middleware, ): DownloaderMiddlewareInterface { if ($middleware instanceof DownloaderMiddlewareInterface) { return $middleware; @@ -55,12 +56,21 @@ public function handleResponse(Response $response): Response return $response; } + public function handleException(Exception $exception, Request $request): ?Request + { + if ($this->middleware instanceof ExceptionMiddlewareInterface) { + return $this->middleware->handleException($exception, $request); + } + + return $request; + } + public function configure(array $options): void { $this->middleware->configure($options); } - public function getMiddleware(): RequestMiddlewareInterface|ResponseMiddlewareInterface + public function getMiddleware(): RequestMiddlewareInterface|ResponseMiddlewareInterface|ExceptionMiddlewareInterface { return $this->middleware; } diff --git a/src/Downloader/Middleware/ExceptionMiddlewareInterface.php b/src/Downloader/Middleware/ExceptionMiddlewareInterface.php new file mode 100644 index 0000000..3938711 --- /dev/null +++ b/src/Downloader/Middleware/ExceptionMiddlewareInterface.php @@ -0,0 +1,14 @@ +exceptionsHandled[] = $exception; + + if (null !== $this->exceptionHandler) { + return ($this->exceptionHandler)($exception, $request); + } + + return $request; + } + public function assertRequestHandled(Request $request): void { Assert::assertContains($request, $this->requestsHandled); @@ -97,4 +115,19 @@ public function assertNoResponseHandled(): void { Assert::assertEmpty($this->responsesHandled); } + + public function assertExceptionHandled(Exception $exception): void + { + Assert::assertContains($exception, $this->exceptionsHandled); + } + + public function assertExceptionNotHandled(Exception $exception): void + { + Assert::assertNotContains($exception, $this->exceptionsHandled); + } + + public function assertNoExceptionHandled(): void + { + Assert::assertEmpty($this->exceptionsHandled); + } } diff --git a/src/Events/ExceptionReceived.php b/src/Events/ExceptionReceived.php new file mode 100644 index 0000000..8226ffc --- /dev/null +++ b/src/Events/ExceptionReceived.php @@ -0,0 +1,17 @@ + + */ + private array $failingRequests = []; + public function pool(array $requests, ?callable $onFulfilled = null, ?callable $onRejected = null): void { foreach ($requests as $request) { $this->sentRequestUrls[] = $request->getUri(); + if (null !== $onRejected && in_array($request, $this->failingRequests)) { + $exception = new RequestException($request, new FakeGuzzleException()); + + $onRejected($exception); + } + if (null !== $onFulfilled) { $response = new Response(new GuzzleResponse(), $request); @@ -39,6 +52,13 @@ public function pool(array $requests, ?callable $onFulfilled = null, ?callable $ } } + public function makeRequestsFail(Request ...$request): static + { + $this->failingRequests = $request; + + return $this; + } + public function assertRequestWasSent(Request $request): void { $uri = $request->getUri(); diff --git a/src/Http/FakeGuzzleException.php b/src/Http/FakeGuzzleException.php new file mode 100644 index 0000000..e6371f1 --- /dev/null +++ b/src/Http/FakeGuzzleException.php @@ -0,0 +1,10 @@ +makeRequest('::url-a::'); $requestB = $this->makeRequest('::url-a::'); - $this->downloader->prepare($requestA); - $this->downloader->prepare($requestB); + $this->downloader->prepare($requestA, null); + $this->downloader->prepare($requestB, null); $this->downloader->flush(); $this->client->assertRequestWasSent($requestA); @@ -69,7 +72,7 @@ public function testPassRequestsThroughRequestHandlersInOrder(): void $this->downloader ->withMiddleware($middlewareA, $middlewareB) - ->prepare($initialRequest); + ->prepare($initialRequest, null); $middlewareA->assertRequestHandled($initialRequest); $middlewareB->assertRequestHandled($middlewareARequest); @@ -83,7 +86,7 @@ public function testDoesNotPassOnRequestIfDroppedByMiddleware(): void $this->downloader ->withMiddleware($dropMiddleware, $middleware) - ->prepare($initialRequest); + ->prepare($initialRequest, null); $dropMiddleware->assertRequestHandled($initialRequest); $middleware->assertNoRequestsHandled(); @@ -96,7 +99,7 @@ public function testDoesNotSendRequestIfDroppedByMiddleware(): void $this->downloader ->withMiddleware($dropMiddleware) - ->prepare($request); + ->prepare($request, null); $this->downloader->flush(); $this->client->assertRequestWasNotSent($request); @@ -111,7 +114,7 @@ public function testSendResponsesThroughMiddlewareInOrder(): void $middlewareC = new FakeMiddleware(); $this->downloader->withMiddleware($middlewareA, $middlewareB, $middlewareC); - $this->downloader->prepare($this->makeRequest()); + $this->downloader->prepare($this->makeRequest(), null); $this->downloader->flush(); $middlewareB->assertResponseHandled($middlewareAResponse); @@ -124,7 +127,7 @@ public function testDontPassOnResponseIfDroppedByMiddleware(): void $middleware = new FakeMiddleware(); $this->downloader->withMiddleware($dropMiddleware, $middleware); - $this->downloader->prepare($this->makeRequest()); + $this->downloader->prepare($this->makeRequest(), null); $this->downloader->flush(); $middleware->assertNoResponseHandled(); @@ -136,8 +139,8 @@ public function testCallResponseCallbackForEachResponse(): void $this->makeRequest('::url-a::')->withMeta('index', 0), $this->makeRequest('::url-b::')->withMeta('index', 1), ]; - $this->downloader->prepare($requests[0]); - $this->downloader->prepare($requests[1]); + $this->downloader->prepare($requests[0], null); + $this->downloader->prepare($requests[1], null); $this->downloader->flush(static function (Response $response) use (&$requests): void { self::assertContains($response->getRequest(), $requests); @@ -152,7 +155,7 @@ public function testDontCallResponseCallbackIfResponseWasDropped(): void $dropMiddleware = new FakeMiddleware(null, static fn (Response $response) => $response->drop('::reason::')); $this->downloader->withMiddleware($dropMiddleware); - $this->downloader->prepare($this->makeRequest()); + $this->downloader->prepare($this->makeRequest(), null); $this->downloader->flush(static function () use (&$called): void { $called = true; }); @@ -166,7 +169,7 @@ public function testDispatchesAnEventIfRequestWasDropped(): void $dropMiddleware = new FakeMiddleware(static fn (Request $request) => $request->drop('::reason::')); $this->downloader->withMiddleware($dropMiddleware); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->dispatcher->assertDispatched( RequestDropped::NAME, @@ -176,7 +179,7 @@ public function testDispatchesAnEventIfRequestWasDropped(): void public function testDoesNotDispatchEventIfRequestWasNotDropped(): void { - $this->downloader->prepare($this->makeRequest()); + $this->downloader->prepare($this->makeRequest(), null); $this->dispatcher->assertNotDispatched(RequestDropped::NAME); } @@ -184,7 +187,7 @@ public function testDoesNotDispatchEventIfRequestWasNotDropped(): void public function testDispatchesAnEventBeforeRequestIsScheduled(): void { $request = $this->makeRequest(); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->dispatcher->assertDispatched( RequestSending::NAME, @@ -199,7 +202,7 @@ public function testDoesNotScheduleEventIfDroppedByEventListener(): void }); $request = $this->makeRequest(); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(); $this->client->assertRequestWasNotSent($request); @@ -212,7 +215,7 @@ public function testDispatchesAnEventIfRequestWasDroppedByListener(): void }); $request = $this->makeRequest(); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->dispatcher->assertDispatched( RequestDropped::NAME, @@ -224,7 +227,7 @@ public function testDispatchEventWhenResponseWasReceived(): void { $request = $this->makeRequest(); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->dispatcher->assertNotDispatched(ResponseReceiving::NAME); $this->downloader->flush(); @@ -266,7 +269,7 @@ public function testDontPassResponseToMiddlewareIfDroppedByExtension(): void $middleware = new FakeMiddleware(); $this->downloader->withMiddleware($middleware); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(); $middleware->assertNoResponseHandled(); @@ -279,7 +282,7 @@ public function testFireEventIfReceivedResponseWasDroppedByExtension(): void $event->response = $event->response->drop('::reason::'); }); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(); $this->dispatcher->assertDispatched( @@ -296,7 +299,7 @@ public function testDontCallParseCallbackIfRequestWasDroppedByExtension(): void $event->response = $event->response->drop('::reason::'); }); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(static function () use (&$called): void { $called = true; }); @@ -316,7 +319,7 @@ public function testDispatchEventWhenResponseWasProcessedByMiddleware(): void ); $this->downloader->withMiddleware($middleware); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(); $this->dispatcher->assertDispatched( @@ -332,7 +335,7 @@ public function testFireEventIfProcessedResponseWasDroppedByExtension(): void $event->response = $event->response->drop('::reason::'); }); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(); $this->dispatcher->assertDispatched( @@ -349,7 +352,7 @@ public function testDontCallParseCallbackIfProcessedResponseWasDroppedByExtensio $event->response = $event->response->drop('::reason::'); }); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(static function () use (&$called): void { $called = true; }); @@ -362,7 +365,7 @@ public function testDontSendRequestIfHasResponse(): void $request = $this->makeRequest(); $request = $request->withResponse($this->makeResponse($request)); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->downloader->flush(); $this->client->assertRequestWasNotSent($request); @@ -373,7 +376,7 @@ public function testResponseDispatchedWhenNotSent(): void $request = $this->makeRequest(); $request = $request->withResponse($this->makeResponse($request)); - $this->downloader->prepare($request); + $this->downloader->prepare($request, null); $this->dispatcher->assertNotDispatched(ResponseReceiving::NAME); $this->downloader->flush(); @@ -394,8 +397,96 @@ public function testPassRequestsThroughRequestHandlersWhenHasResponse(): void $this->downloader ->withMiddleware($middleware) - ->prepare($request); + ->prepare($request, null); $middleware->assertRequestHandled($request); } + + public function testCallsOnRejectedCallbackWhenExceptionOccursDuringRequestHandling(): void + { + $called = false; + $request = $this->makeRequest(); + $exceptionMiddleware = new FakeMiddleware(static fn (Request $request) => throw new \Exception('Oh no!')); + + $this->downloader + ->withMiddleware($exceptionMiddleware) + ->prepare($request, function () use (&$called) { + $called = true; + }); + $this->downloader->flush(); + + $this->client->assertRequestWasNotSent($request); + self::assertTrue($called); + } + + public function testCallsOnRejectedCallbackWhenExceptionOccursDuringFlushing(): void + { + $called = false; + $request = $this->makeRequest(); + $this->client->makeRequestsFail($request); + + $this->downloader->prepare($request, null); + $this->downloader->flush(onRejected: function () use (&$called) { + $called = true; + }); + + self::assertTrue($called); + } + + public function testPassExceptionsThroughExceptionHandlers(): void + { + $request = $this->makeRequest(); + $exception = new Exception('Oh no!'); + $exceptionMiddleware = new FakeMiddleware(static fn () => throw $exception); + + $this->downloader + ->withMiddleware($exceptionMiddleware) + ->prepare($request, null); + + $exceptionMiddleware->assertExceptionHandled($exception); + } + + public function testDispatchEventIfExceptionIsBeingReceived(): void + { + $request = $this->makeRequest(); + $successfulMiddleware = new FakeMiddleware(); + $exception = new Exception('On no!'); + $failingMiddleware = new FakeMiddleware(static fn () => throw $exception); + + $this->downloader + ->withMiddleware($successfulMiddleware) + ->prepare($request, null); + $this->dispatcher->assertNotDispatched(ExceptionReceiving::class); + + $this->downloader + ->withMiddleware($failingMiddleware) + ->prepare($request, null); + + $this->dispatcher->assertDispatched( + ExceptionReceiving::NAME, + static fn (ExceptionReceiving $event) => $event->exception == $exception, + ); + } + + public function testDispatchEventWhenExceptionWasProcessedByMiddleware(): void + { + $request = $this->makeRequest(); + $exception = new Exception('On no!'); + $middleware = new FakeMiddleware( + requestHandler: static fn () => throw $exception, + exceptionHandler: function (Exception $exception, Request $request) { + $this->dispatcher->assertNotDispatched(ExceptionReceived::NAME); + + return $request; + }, + ); + $this->downloader->withMiddleware($middleware); + + $this->downloader->prepare($request, null); + + $this->dispatcher->assertDispatched( + ExceptionReceived::NAME, + static fn (ExceptionReceived $event) => $event->exception === $exception, + ); + } } From 6d8faeac676d869c68adbfcc07b99a6301771e72 Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Mon, 17 Mar 2025 08:31:33 +0100 Subject: [PATCH 2/8] Log when exception received --- src/Extensions/LoggerExtension.php | 9 +++++++++ tests/Extensions/LoggerExtensionTest.php | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/Extensions/LoggerExtension.php b/src/Extensions/LoggerExtension.php index 49ad990..cb07b24 100644 --- a/src/Extensions/LoggerExtension.php +++ b/src/Extensions/LoggerExtension.php @@ -14,6 +14,7 @@ namespace RoachPHP\Extensions; use Psr\Log\LoggerInterface; +use RoachPHP\Events\ExceptionReceived; use RoachPHP\Events\ItemDropped; use RoachPHP\Events\ItemScraped; use RoachPHP\Events\RequestDropped; @@ -39,6 +40,7 @@ public static function getSubscribedEvents(): array RequestDropped::NAME => ['onRequestDropped', 100], ItemScraped::NAME => ['onItemScraped', 100], ItemDropped::NAME => ['onItemDropped', 100], + ExceptionReceived::NAME => ['onExceptionReceived', 100], ]; } @@ -81,4 +83,11 @@ public function onItemDropped(ItemDropped $event): void 'reason' => $event->item->getDropReason(), ]); } + + public function onExceptionReceived(ExceptionReceived $event): void + { + $this->logger->warning('Exception received', [ + 'exception' => $event->exception, + ]); + } } diff --git a/tests/Extensions/LoggerExtensionTest.php b/tests/Extensions/LoggerExtensionTest.php index 99de5a9..30290de 100644 --- a/tests/Extensions/LoggerExtensionTest.php +++ b/tests/Extensions/LoggerExtensionTest.php @@ -13,7 +13,9 @@ namespace RoachPHP\Tests\Extensions; +use Exception; use RoachPHP\Core\Run; +use RoachPHP\Events\ExceptionReceived; use RoachPHP\Events\ItemDropped; use RoachPHP\Events\ItemScraped; use RoachPHP\Events\RequestDropped; @@ -128,6 +130,20 @@ public function testLogWhenItemWasScraped(): void ); } + public function testLogWhenExceptionWasReceived(): void + { + self::assertFalse( + $this->logger->messageWasLogged('warning', 'Exception received'), + ); + + $exception = new Exception(); + $this->dispatch(new ExceptionReceived($exception), ExceptionReceived::NAME); + + self::assertTrue( + $this->logger->messageWasLogged('warning', 'Exception received', ['exception' => $exception]), + ); + } + protected function createExtension(): ExtensionInterface { $this->logger = new FakeLogger(); From c5975b5b9a7370bb68f6deddb75e9e70d6ec37de Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Sun, 23 Mar 2025 20:48:38 +0100 Subject: [PATCH 3/8] Fix test --- src/Http/FakeGuzzleException.php | 2 ++ tests/Downloader/DownloaderMiddlewareAdapterTest.php | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/Http/FakeGuzzleException.php b/src/Http/FakeGuzzleException.php index e6371f1..85e50aa 100644 --- a/src/Http/FakeGuzzleException.php +++ b/src/Http/FakeGuzzleException.php @@ -1,5 +1,7 @@ Date: Wed, 16 Apr 2025 16:38:01 +0200 Subject: [PATCH 4/8] Set null default for onRejected callback --- src/Downloader/Downloader.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Downloader/Downloader.php b/src/Downloader/Downloader.php index b9da613..70544e7 100644 --- a/src/Downloader/Downloader.php +++ b/src/Downloader/Downloader.php @@ -57,7 +57,7 @@ public function scheduledRequests(): int return \count($this->requests); } - public function prepare(Request $request, ?callable $onRejected): void + public function prepare(Request $request, ?callable $onRejected = null): void { try { foreach ($this->middleware as $middleware) { From 304bddcf3ea3fcdd854695be6f4504154081ce4a Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Sat, 26 Apr 2025 12:04:48 +0200 Subject: [PATCH 5/8] Allow other exceptions besides guzzle exception in RequestException --- src/Http/RequestException.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/RequestException.php b/src/Http/RequestException.php index f3c37fd..57e16c3 100644 --- a/src/Http/RequestException.php +++ b/src/Http/RequestException.php @@ -19,7 +19,7 @@ final class RequestException extends \Exception { public function __construct( private Request $request, - private GuzzleException $reason, + private GuzzleException|\Exception $reason, ) { parent::__construct('An exception occurred while sending a request', previous: $reason); } @@ -29,7 +29,7 @@ public function getRequest(): Request return $this->request; } - public function getReason(): GuzzleException + public function getReason(): GuzzleException|\Exception { return $this->reason; } From 2f5434d1b9e2a1529f53947480c52c3604bf6226 Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Sat, 26 Apr 2025 12:05:33 +0200 Subject: [PATCH 6/8] Track if RequestException is handled or not --- src/Http/RequestException.php | 14 ++++++++++++++ tests/Http/RequestExceptionTest.php | 30 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tests/Http/RequestExceptionTest.php diff --git a/src/Http/RequestException.php b/src/Http/RequestException.php index 57e16c3..cb9e35f 100644 --- a/src/Http/RequestException.php +++ b/src/Http/RequestException.php @@ -17,6 +17,8 @@ final class RequestException extends \Exception { + private bool $handled = false; + public function __construct( private Request $request, private GuzzleException|\Exception $reason, @@ -33,4 +35,16 @@ public function getReason(): GuzzleException|\Exception { return $this->reason; } + + public function isHandled(): bool + { + return $this->handled; + } + + public function setHandled(): self + { + $this->handled = true; + + return $this; + } } diff --git a/tests/Http/RequestExceptionTest.php b/tests/Http/RequestExceptionTest.php new file mode 100644 index 0000000..6ff24c3 --- /dev/null +++ b/tests/Http/RequestExceptionTest.php @@ -0,0 +1,30 @@ +makeRequest(), new \Exception()); + + $this->assertFalse($exception->isHandled()); + + $exception = $exception->setHandled(); + $this->assertTrue($exception->isHandled()); + } +} From ce24d2d5ad42ad521defdd752457509b50399763 Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Sat, 26 Apr 2025 12:38:31 +0200 Subject: [PATCH 7/8] Make exception handler method in Downloader expect RequestException --- src/Downloader/Downloader.php | 18 +++++++----------- .../Middleware/DownloaderMiddlewareAdapter.php | 7 ++++--- .../ExceptionMiddlewareInterface.php | 5 ++--- src/Downloader/Middleware/FakeMiddleware.php | 11 +++++++---- .../DownloaderMiddlewareAdapterTest.php | 5 +++-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/Downloader/Downloader.php b/src/Downloader/Downloader.php index 70544e7..7e4c370 100644 --- a/src/Downloader/Downloader.php +++ b/src/Downloader/Downloader.php @@ -94,7 +94,7 @@ public function prepare(Request $request, ?callable $onRejected = null): void $this->requests[] = $event->request; } catch (Exception $exception) { - $this->onExceptionReceived($exception, $request, $onRejected); + $this->onExceptionReceived(new RequestException($request, $exception), $onRejected); } } @@ -122,11 +122,7 @@ function (Response $response) use ($onFullFilled): void { $this->onResponseReceived($response, $onFullFilled); }, function (RequestException $requestException) use ($onRejected): void { - $this->onExceptionReceived( - $requestException->getReason(), - $requestException->getRequest(), - $onRejected, - ); + $this->onExceptionReceived($requestException, $onRejected); } ); } @@ -177,24 +173,24 @@ private function onResponseReceived(Response $response, ?callable $callback): vo } } - private function onExceptionReceived(\Throwable $exception, Request $request, ?callable $callback): void + private function onExceptionReceived(RequestException $requestException, ?callable $callback): void { $this->eventDispatcher->dispatch( - new ExceptionReceiving($exception), + new ExceptionReceiving($requestException), ExceptionReceiving::NAME, ); foreach ($this->middleware as $middleware) { - $request = $middleware->handleException($exception, $request); + $middleware->handleException($requestException); } $this->eventDispatcher->dispatch( - new ExceptionReceived($exception), + new ExceptionReceived($requestException), ExceptionReceived::NAME, ); if (null !== $callback) { - $callback($exception, $request); + $callback($requestException); } } } diff --git a/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php b/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php index b86a2e4..b270e25 100644 --- a/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php +++ b/src/Downloader/Middleware/DownloaderMiddlewareAdapter.php @@ -16,6 +16,7 @@ use Exception; use RoachPHP\Downloader\DownloaderMiddlewareInterface; use RoachPHP\Http\Request; +use RoachPHP\Http\RequestException; use RoachPHP\Http\Response; /** @@ -56,13 +57,13 @@ public function handleResponse(Response $response): Response return $response; } - public function handleException(Exception $exception, Request $request): ?Request + public function handleException(RequestException $requestException): RequestException { if ($this->middleware instanceof ExceptionMiddlewareInterface) { - return $this->middleware->handleException($exception, $request); + return $this->middleware->handleException($requestException); } - return $request; + return $requestException; } public function configure(array $options): void diff --git a/src/Downloader/Middleware/ExceptionMiddlewareInterface.php b/src/Downloader/Middleware/ExceptionMiddlewareInterface.php index 3938711..a36561b 100644 --- a/src/Downloader/Middleware/ExceptionMiddlewareInterface.php +++ b/src/Downloader/Middleware/ExceptionMiddlewareInterface.php @@ -4,11 +4,10 @@ namespace RoachPHP\Downloader\Middleware; -use Exception; -use RoachPHP\Http\Request; +use RoachPHP\Http\RequestException; use RoachPHP\Support\ConfigurableInterface; interface ExceptionMiddlewareInterface extends ConfigurableInterface { - public function handleException(Exception $exception, Request $request): ?Request; + public function handleException(RequestException $requestException): RequestException; } diff --git a/src/Downloader/Middleware/FakeMiddleware.php b/src/Downloader/Middleware/FakeMiddleware.php index 8bffb9f..e2b269a 100644 --- a/src/Downloader/Middleware/FakeMiddleware.php +++ b/src/Downloader/Middleware/FakeMiddleware.php @@ -16,7 +16,9 @@ use Exception; use PHPUnit\Framework\Assert; use RoachPHP\Downloader\DownloaderMiddlewareInterface; +use RoachPHP\Http\ExceptionContext; use RoachPHP\Http\Request; +use RoachPHP\Http\RequestException; use RoachPHP\Http\Response; use RoachPHP\Support\Configurable; @@ -45,6 +47,7 @@ final class FakeMiddleware implements DownloaderMiddlewareInterface /** * @param ?\Closure(Request): Request $requestHandler * @param ?\Closure(Response): Response $responseHandler + * @param ?\Closure(RequestException): RequestException $exceptionHandler */ public function __construct( private ?\Closure $requestHandler = null, @@ -75,15 +78,15 @@ public function handleResponse(Response $response): Response return $response; } - public function handleException(Exception $exception, Request $request): ?Request + public function handleException(RequestException $requestException): RequestException { - $this->exceptionsHandled[] = $exception; + $this->exceptionsHandled[] = $requestException->getReason(); if (null !== $this->exceptionHandler) { - return ($this->exceptionHandler)($exception, $request); + return ($this->exceptionHandler)($requestException); } - return $request; + return $requestException; } public function assertRequestHandled(Request $request): void diff --git a/tests/Downloader/DownloaderMiddlewareAdapterTest.php b/tests/Downloader/DownloaderMiddlewareAdapterTest.php index 9a08be1..ed70e8d 100644 --- a/tests/Downloader/DownloaderMiddlewareAdapterTest.php +++ b/tests/Downloader/DownloaderMiddlewareAdapterTest.php @@ -20,6 +20,7 @@ use RoachPHP\Downloader\Middleware\RequestMiddlewareInterface; use RoachPHP\Downloader\Middleware\ResponseMiddlewareInterface; use RoachPHP\Http\Request; +use RoachPHP\Http\RequestException; use RoachPHP\Http\Response; use RoachPHP\Support\Configurable; use RoachPHP\Testing\Concerns\InteractsWithRequestsAndResponses; @@ -46,9 +47,9 @@ public function handleResponse(Response $response): Response return $response; } - public function handleException(Exception $exception, Request $request): ?Request + public function handleException(RequestException $requestException): RequestException { - return $request; + return $requestException; } }; From 1494d053ad480b885889cdcaf29de15ee47677af Mon Sep 17 00:00:00 2001 From: Fadarrizz Date: Sat, 26 Apr 2025 12:38:57 +0200 Subject: [PATCH 8/8] Throw exception when unhandled by any middleware --- src/Downloader/Downloader.php | 8 ++++++ tests/Downloader/DownloaderTest.php | 42 ++++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/Downloader/Downloader.php b/src/Downloader/Downloader.php index 7e4c370..0a3960b 100644 --- a/src/Downloader/Downloader.php +++ b/src/Downloader/Downloader.php @@ -180,8 +180,13 @@ private function onExceptionReceived(RequestException $requestException, ?callab ExceptionReceiving::NAME, ); + $handled = false; foreach ($this->middleware as $middleware) { $middleware->handleException($requestException); + + if ($requestException->isHandled()) { + $handled = true; + } } $this->eventDispatcher->dispatch( @@ -191,6 +196,9 @@ private function onExceptionReceived(RequestException $requestException, ?callab if (null !== $callback) { $callback($requestException); + + } else if (!$handled) { + throw $requestException; } } } diff --git a/tests/Downloader/DownloaderTest.php b/tests/Downloader/DownloaderTest.php index 488eba3..47b0b69 100644 --- a/tests/Downloader/DownloaderTest.php +++ b/tests/Downloader/DownloaderTest.php @@ -27,6 +27,7 @@ use RoachPHP\Events\ResponseReceiving; use RoachPHP\Http\FakeClient; use RoachPHP\Http\Request; +use RoachPHP\Http\RequestException; use RoachPHP\Http\Response; use RoachPHP\Testing\Concerns\InteractsWithRequestsAndResponses; @@ -406,7 +407,11 @@ public function testCallsOnRejectedCallbackWhenExceptionOccursDuringRequestHandl { $called = false; $request = $this->makeRequest(); - $exceptionMiddleware = new FakeMiddleware(static fn (Request $request) => throw new \Exception('Oh no!')); + $exceptionMiddleware = new FakeMiddleware( + requestHandler: static fn () => throw new \Exception('Oh no!'), + // Handle request exception to not let the exception be thrown + exceptionHandler: static fn (RequestException $requestException) => $requestException->setHandled() + ); $this->downloader ->withMiddleware($exceptionMiddleware) @@ -437,7 +442,11 @@ public function testPassExceptionsThroughExceptionHandlers(): void { $request = $this->makeRequest(); $exception = new Exception('Oh no!'); - $exceptionMiddleware = new FakeMiddleware(static fn () => throw $exception); + $exceptionMiddleware = new FakeMiddleware( + requestHandler: static fn () => throw $exception, + // Handle request exception to not let the exception be thrown + exceptionHandler: static fn (RequestException $requestException) => $requestException->setHandled() + ); $this->downloader ->withMiddleware($exceptionMiddleware) @@ -451,7 +460,11 @@ public function testDispatchEventIfExceptionIsBeingReceived(): void $request = $this->makeRequest(); $successfulMiddleware = new FakeMiddleware(); $exception = new Exception('On no!'); - $failingMiddleware = new FakeMiddleware(static fn () => throw $exception); + $failingMiddleware = new FakeMiddleware( + requestHandler: static fn () => throw $exception, + // Handle request exception to not let the exception be thrown + exceptionHandler: static fn (RequestException $requestException) => $requestException->setHandled() + ); $this->downloader ->withMiddleware($successfulMiddleware) @@ -464,7 +477,7 @@ public function testDispatchEventIfExceptionIsBeingReceived(): void $this->dispatcher->assertDispatched( ExceptionReceiving::NAME, - static fn (ExceptionReceiving $event) => $event->exception == $exception, + static fn (ExceptionReceiving $event) => $event->exception->getPrevious() == $exception, ); } @@ -474,10 +487,11 @@ public function testDispatchEventWhenExceptionWasProcessedByMiddleware(): void $exception = new Exception('On no!'); $middleware = new FakeMiddleware( requestHandler: static fn () => throw $exception, - exceptionHandler: function (Exception $exception, Request $request) { + exceptionHandler: function (RequestException $requestException) { $this->dispatcher->assertNotDispatched(ExceptionReceived::NAME); - return $request; + // Handle request exception to not let the exception be thrown + return $requestException->setHandled(); }, ); $this->downloader->withMiddleware($middleware); @@ -486,7 +500,21 @@ public function testDispatchEventWhenExceptionWasProcessedByMiddleware(): void $this->dispatcher->assertDispatched( ExceptionReceived::NAME, - static fn (ExceptionReceived $event) => $event->exception === $exception, + static fn (ExceptionReceived $event) => $event->exception->getPrevious() === $exception, ); } + + public function testThrowsExceptionWhenNoMiddlewareSpecified(): void + { + $request = $this->makeRequest(); + $exception = new Exception('On no!'); + $middleware = new FakeMiddleware(requestHandler: static fn () => throw $exception); + $this->downloader->withMiddleware($middleware); + + try { + $this->downloader->prepare($request, null); + } catch (RequestException $requestException) { + $this->assertEquals($exception, $requestException->getPrevious()); + } + } }