From 72d37172a0a6c702882f91915dc4460e3801a28e Mon Sep 17 00:00:00 2001 From: Shoxcie Date: Wed, 29 Apr 2026 11:52:35 +0300 Subject: [PATCH 1/5] feat!: add result arg to onSuccess callback --- CHANGELOG.md | 8 ++++++ CLAUDE.md | 4 +-- README.md | 4 +-- UPGRADE-4.0.md | 45 ++++++++++++++++++++++++++++++ example.php | 2 +- src/BatchHttpClient.php | 6 ++-- tests/Unit/BatchHttpClientTest.php | 35 +++++++++++++++++++---- 7 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 UPGRADE-4.0.md diff --git a/CHANGELOG.md b/CHANGELOG.md index edad6a0..bf80e27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [4.0.0] - 2026-04-29 + +### Changed + +- `BatchHttpClient::onSuccess()` callback signature is now `Closure(string $key, mixed $result, ResponseInterface $response): void` (previously `Closure(string $key, ResponseInterface $response): void`). The `$result` is the value stored in `$results[$key]` — i.e. post-`parseResponse` if one is configured. Not contravariant: existing closures must add the `$result` parameter. + +See [UPGRADE-4.0.md](UPGRADE-4.0.md) for migration details. + ## [3.1.0] - 2026-04-28 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index e0de721..c623768 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ $results = $client 'users' => new RequestConfig('GET', 'https://api.example.com/users', maxRetries: 3), 'orders' => new RequestConfig('POST', 'https://api.example.com/orders', options: ['json' => $data]), ]) - ->onSuccess(function (string $key, ResponseInterface $response) { ... }) + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) { ... }) ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { ... }) ->onExhausted(function (string $key, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e) { ... }) ->onAbort(function (string $key, ResponseInterface $response, Throwable $e) { ... }) @@ -57,7 +57,7 @@ Fires all HTTP requests immediately (Symfony HttpClient is async by default). St ### Callbacks -- `onSuccess(Closure)` — called for each 2xx response after `parseResponse` (if any) returns: `(string $key, ResponseInterface $response)` +- `onSuccess(Closure)` — called for each 2xx response after `parseResponse` (if any) returns: `(string $key, mixed $result, ResponseInterface $response)`. `$result` is the value stored in `$results[$key]` (post-`parseResponse` if configured). - `onRetry(Closure)` — called when a retry fires: `(string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse)` - `onExhausted(Closure)` — called when a single request exhausts all retries: `(string $key, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e)` - `onAbort(Closure)` — called when an unexpected exception cancels the whole batch (broken JSON, throwing user callback, parser throwing something other than `InvalidResponseException`, etc.): `(string $key, ResponseInterface $response, Throwable $e)`. Skipped if the throw happened before any response was processed. diff --git a/README.md b/README.md index 7d8320e..942404b 100644 --- a/README.md +++ b/README.md @@ -79,8 +79,8 @@ Retry options are merged onto the original options via `array_replace_recursive( ```php $results = $client ->request([...]) - ->onSuccess(function (string $key, ResponseInterface $response) { - // called for each 2xx response + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) { + // called for each 2xx response, after parseResponse if configured }) ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { // called when a retry fires diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md new file mode 100644 index 0000000..fa7a641 --- /dev/null +++ b/UPGRADE-4.0.md @@ -0,0 +1,45 @@ +# Upgrading to 4.0 + +Breaking callback signature change. + +## 1. `onSuccess()` callback receives the parsed result + +`onSuccess` now gets the value already stored in the results array as a third positional argument, mirroring `parseResponse`. Callbacks no longer need to call `$response->toArray()` again to inspect the body, and they automatically see whatever `parseResponse` returned (if one is configured). + +This is **not** contravariant — every existing `onSuccess` closure must add the new `mixed $result` parameter between `$key` and `$response`. + +| Before | After | +|---|---| +| `onSuccess(fn(string $key, ResponseInterface $r))` | `onSuccess(fn(string $key, mixed $result, ResponseInterface $r))` | + +```php +// before +new BatchHttpClient() + ->request([...]) + ->onSuccess(function (string $key, ResponseInterface $response): void { + $body = $response->toArray(); // re-decoded the body just to read it + log_success($key, $body); + }) + ->fetch(); + +// after +new BatchHttpClient() + ->request([...]) + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { + // $result is what's already in $results[$key] — post-parseResponse if configured + log_success($key, $result); + }) + ->fetch(); +``` + +If a `RequestConfig` configured a `parseResponse` closure, `$result` is its return value (i.e. the same value `fetch()` returns in `$results[$key]`). With no `parseResponse`, `$result` is `array` (when `decodeJson: true`, the default) or `string` (when `decodeJson: false`). + +## Finding callers before you upgrade + +Grep for the call: + +``` +grep -rn '->onSuccess(' src tests +``` + +Each match must add the `$result` parameter to its closure signature. diff --git a/example.php b/example.php index 870d8ca..040e157 100644 --- a/example.php +++ b/example.php @@ -60,7 +60,7 @@ function simpleLog( echo implode(' | ', $parts) . PHP_EOL . PHP_EOL; } -function logSuccess(string $key, ResponseInterface $response): void +function logSuccess(string $key, mixed $result, ResponseInterface $response): void { simpleLog( prefix: 'SUCCESS', diff --git a/src/BatchHttpClient.php b/src/BatchHttpClient.php index a91ab00..61f2cee 100644 --- a/src/BatchHttpClient.php +++ b/src/BatchHttpClient.php @@ -29,7 +29,7 @@ final class BatchHttpClient /** @var array */ private array $results = []; - /** @var null|Closure(string, ResponseInterface): void */ + /** @var null|Closure(string, mixed, ResponseInterface): void */ private ?Closure $onSuccess = null; /** @var null|Closure(string, int, ResponseInterface, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException, ResponseInterface): void */ @@ -77,7 +77,7 @@ public function request(array $configs): static return $this; } - /** @param Closure(string, ResponseInterface): void $closure */ + /** @param Closure(string, mixed, ResponseInterface): void $closure */ public function onSuccess(Closure $closure): static { $this->onSuccess = $closure; @@ -140,7 +140,7 @@ public function fetch(): array $this->results[$key] = $result; if ($this->onSuccess instanceof Closure) { - ($this->onSuccess)($key, $response); + ($this->onSuccess)($key, $result, $response); } unset($this->responses[$key]); diff --git a/tests/Unit/BatchHttpClientTest.php b/tests/Unit/BatchHttpClientTest.php index 59c57ff..37e4877 100644 --- a/tests/Unit/BatchHttpClientTest.php +++ b/tests/Unit/BatchHttpClientTest.php @@ -465,7 +465,7 @@ function (string $method, string $url): JsonMockResponse { }); describe('callbacks', function (): void { - test('onSuccess receives key and response', function (): void { + test('onSuccess receives key, result, and response', function (): void { $captured = []; $mockClient = new MockHttpClient([ @@ -478,15 +478,17 @@ function (string $method, string $url): JsonMockResponse { 'users' => new RequestConfig('GET', 'https://api.example.com/users'), 'orders' => new RequestConfig('GET', 'https://api.example.com/orders'), ]) - ->onSuccess(function (string $key, ResponseInterface $response) use (&$captured): void { - $captured[] = ['key' => $key, 'response' => $response]; + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) use (&$captured): void { + $captured[] = ['key' => $key, 'result' => $result, 'response' => $response]; }) ->fetch(); expect($captured)->toHaveCount(2) ->and($captured[0]['key'])->toBe('users') + ->and($captured[0]['result'])->toBe(['id' => 1]) ->and($captured[0]['response'])->toBeInstanceOf(ResponseInterface::class) ->and($captured[1]['key'])->toBe('orders') + ->and($captured[1]['result'])->toBe(['id' => 2]) ->and($captured[1]['response'])->toBeInstanceOf(ResponseInterface::class); }); @@ -921,7 +923,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & ->request([ 'api' => new RequestConfig('GET', 'https://api.example.com/api'), ]) - ->onSuccess(function (string $key, ResponseInterface $response): void { + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { throw new \RuntimeException('boom'); }) ->fetch(), @@ -942,7 +944,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & 'b' => new RequestConfig('GET', 'https://api.example.com/b', maxRetries: 3), 'c' => new RequestConfig('GET', 'https://api.example.com/c', maxRetries: 3), ]) - ->onSuccess(function (string $key, ResponseInterface $response): void { + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { throw new \RuntimeException('boom'); }) ->fetch(), @@ -971,6 +973,29 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & expect($results['api'])->toBe(['id' => 1, 'name' => 'Alice']); }); + test('onSuccess receives the parseResponse-transformed result, not the raw decoded body', function (): void { + $captured = null; + + $mockClient = new MockHttpClient([ + new JsonMockResponse(['data' => ['id' => 42], 'meta' => ['v' => 1]]), + ]); + + new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig( + 'GET', + 'https://api.example.com/api', + parseResponse: fn(string $key, mixed $result, ResponseInterface $response): mixed => $result['data'], + ), + ]) + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) use (&$captured): void { + $captured = $result; + }) + ->fetch(); + + expect($captured)->toBe(['id' => 42]); + }); + test('receives key, decoded result, and response', function (): void { $captured = null; From 134b6cb2c4b46d441af08c1ad9b89b707120215e Mon Sep 17 00:00:00 2001 From: Shoxcie Date: Wed, 29 Apr 2026 12:03:57 +0300 Subject: [PATCH 2/5] chore!: move upgrade guides to upgrade/ folder --- CHANGELOG.md | 6 +++--- README.md | 2 +- UPGRADE-2.0.md => upgrade/2.0.md | 0 UPGRADE-3.0.md => upgrade/3.0.md | 0 UPGRADE-4.0.md => upgrade/4.0.md | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename UPGRADE-2.0.md => upgrade/2.0.md (100%) rename UPGRADE-3.0.md => upgrade/3.0.md (100%) rename UPGRADE-4.0.md => upgrade/4.0.md (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf80e27..07f3b78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `BatchHttpClient::onSuccess()` callback signature is now `Closure(string $key, mixed $result, ResponseInterface $response): void` (previously `Closure(string $key, ResponseInterface $response): void`). The `$result` is the value stored in `$results[$key]` — i.e. post-`parseResponse` if one is configured. Not contravariant: existing closures must add the `$result` parameter. -See [UPGRADE-4.0.md](UPGRADE-4.0.md) for migration details. +See [upgrade/4.0.md](upgrade/4.0.md) for migration details. ## [3.1.0] - 2026-04-28 @@ -31,7 +31,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for migration details. - `BatchHttpClient::onFailure(Closure)` (deprecated in 2.1.0). Replace with `onExhausted()` for the retries-exhausted path and/or `onAbort()` for the unexpected-abort path. -See [UPGRADE-3.0.md](UPGRADE-3.0.md) for migration details. +See [upgrade/3.0.md](upgrade/3.0.md) for migration details. ## [2.1.0] - 2026-04-22 @@ -56,7 +56,7 @@ See [UPGRADE-3.0.md](UPGRADE-3.0.md) for migration details. - `BatchHttpClient::request()` now throws `InvalidArgumentException` if `RequestConfig::$options` contains the reserved `user_data` key. - `BatchHttpClient::fetch()` now throws `InvalidArgumentException` if a `retryOptions` Closure returns options containing `user_data`. -See [UPGRADE-2.0.md](UPGRADE-2.0.md) for migration details. +See [upgrade/2.0.md](upgrade/2.0.md) for migration details. ## [1.1.0] - 2026-04-20 diff --git a/README.md b/README.md index 942404b..7fc7b91 100644 --- a/README.md +++ b/README.md @@ -187,7 +187,7 @@ $client = new BatchHttpClient($httpClient); ## Upgrading -See [UPGRADE-2.0.md](UPGRADE-2.0.md) for the `1.x` → `2.x` migration guide. +See the [upgrade/](upgrade/) folder for migration guides between major versions. ## License diff --git a/UPGRADE-2.0.md b/upgrade/2.0.md similarity index 100% rename from UPGRADE-2.0.md rename to upgrade/2.0.md diff --git a/UPGRADE-3.0.md b/upgrade/3.0.md similarity index 100% rename from UPGRADE-3.0.md rename to upgrade/3.0.md diff --git a/UPGRADE-4.0.md b/upgrade/4.0.md similarity index 100% rename from UPGRADE-4.0.md rename to upgrade/4.0.md From d8a2e2dab8e313df595735e47d8c5f4fa377cc8f Mon Sep 17 00:00:00 2001 From: Shoxcie Date: Wed, 29 Apr 2026 12:20:59 +0300 Subject: [PATCH 3/5] refactor!: rename throwOnError to throwOnExhausted --- CHANGELOG.md | 1 + CLAUDE.md | 14 +++---- README.md | 10 ++--- example.php | 4 +- src/BatchHttpClient.php | 4 +- src/RequestConfig.php | 2 +- tests/Unit/BatchHttpClientTest.php | 64 +++++++++++++++--------------- upgrade/4.0.md | 31 +++++++++++++-- 8 files changed, 78 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f3b78..497fa0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - `BatchHttpClient::onSuccess()` callback signature is now `Closure(string $key, mixed $result, ResponseInterface $response): void` (previously `Closure(string $key, ResponseInterface $response): void`). The `$result` is the value stored in `$results[$key]` — i.e. post-`parseResponse` if one is configured. Not contravariant: existing closures must add the `$result` parameter. +- `RequestConfig::$throwOnError` renamed to `RequestConfig::$throwOnExhausted`. The previous name suggested "throw on any error" but the property only governs the post-retry-exhausted path; the new name aligns with the existing `onExhausted` callback. No behavior change. See [upgrade/4.0.md](upgrade/4.0.md) for migration details. diff --git a/CLAUDE.md b/CLAUDE.md index c623768..bb466f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,11 +35,11 @@ $results = $client - `url` (string) - `options` (array, default `[]`) — standard Symfony HttpClient options (timeout, max_duration, headers, etc.) - `retryOptions` (array|Closure, default `[]`) — merged onto options for retries via `array_replace_recursive($options, $retryOptions)`. If Closure: receives `(int $attempt, Throwable $e)`, must return options array. - - `throwOnError` (bool, default `true`) — if true and request exhausts all retries, rethrow last exception and cancel all in-flight requests + - `throwOnExhausted` (bool, default `true`) — if true and request exhausts all retries, rethrow last exception and cancel all in-flight requests - `decodeJson` (bool, default `true`) — `true`: `toArray()`, `false`: `getContent()` - `maxRetries` (int, default `0`) — max retry count - `retryOnTransportException` (bool, default `true`) — whether to retry on Symfony transport exceptions (connection timeouts, DNS failures, etc.) - - `parseResponse` (Closure|null, default `null`) — runs after the body is decoded and before `onSuccess`, only on a 2xx response. Receives `(string $key, mixed $result, ResponseInterface $response)` and returns the value to store in `$results[$key]`. Throwing `InvalidResponseException` triggers a retry just like an HTTP/transport failure (counts against `maxRetries`, fires `onRetry`, and on exhaustion fires `onExhausted` + rethrows if `throwOnError: true`). + - `parseResponse` (Closure|null, default `null`) — runs after the body is decoded and before `onSuccess`, only on a 2xx response. Receives `(string $key, mixed $result, ResponseInterface $response)` and returns the value to store in `$results[$key]`. Throwing `InvalidResponseException` triggers a retry just like an HTTP/transport failure (counts against `maxRetries`, fires `onRetry`, and on exhaustion fires `onExhausted` + rethrows if `throwOnExhausted: true`). - `InvalidResponseException` — marker `RuntimeException` thrown from a `parseResponse` closure to reject a semantically invalid 2xx response and request a retry. ### `request(array)` — fire all requests, return `static` @@ -76,8 +76,8 @@ Fires all HTTP requests immediately (Symfony HttpClient is async by default). St ### Output - `array` of results keyed by request key -- Failed optional requests (`throwOnError: false`) return `null` -- If `throwOnError` request fails after all retries → rethrow the last caught Symfony exception, cancel all remaining requests +- Failed optional requests (`throwOnExhausted: false`) return `null` +- If `throwOnExhausted` request fails after all retries → rethrow the last caught Symfony exception, cancel all remaining requests ## Commands @@ -108,8 +108,8 @@ Unit tests in `tests/Unit/BatchHttpClientTest.php` using `MockHttpClient` / `Moc - Successful batch requests (2xx) — verify results array matches input keys - Mixed success/failure results — some 2xx, some errors - Retry behavior — verify retry count -- `throwOnError: true` — exception thrown after retries exhausted, all in-flight cancelled -- `throwOnError: false` — failed requests return `null` +- `throwOnExhausted: true` — exception thrown after retries exhausted, all in-flight cancelled +- `throwOnExhausted: false` — failed requests return `null` - Transport exception handling — DNS failure, connection timeout - `retryOnTransportException: true` vs `false` - `onSuccess` / `onRetry` / `onExhausted` / `onAbort` callbacks — verify they receive correct arguments @@ -118,4 +118,4 @@ Unit tests in `tests/Unit/BatchHttpClientTest.php` using `MockHttpClient` / `Moc - `retryOptions` as Closure — verify dynamic retry options based on attempt/exception - `user_data` rejection — throws `InvalidArgumentException` when `options` or a `retryOptions` Closure contains `user_data` - Safety-net catch — outer `catch(Throwable)` handles unexpected exceptions (e.g. from callbacks, broken JSON with `decodeJson: true`) -- `parseResponse` — replaces results, retries on `InvalidResponseException`, exhausts to `onExhausted`/`null`/rethrow per `throwOnError`, and routes any other throwable to `onAbort` +- `parseResponse` — replaces results, retries on `InvalidResponseException`, exhausts to `onExhausted`/`null`/rethrow per `throwOnExhausted`, and routes any other throwable to `onAbort` diff --git a/README.md b/README.md index 7fc7b91..749d9a6 100644 --- a/README.md +++ b/README.md @@ -101,15 +101,15 @@ By default, if a request exhausts all retries, the last exception is rethrown an ```php new RequestConfig('GET', 'https://api.example.com/critical', maxRetries: 3, - throwOnError: true, // default + throwOnExhausted: true, // default ) ``` -Set `throwOnError: false` for optional requests. Failed optional requests return `null` in the results array: +Set `throwOnExhausted: false` for optional requests. Failed optional requests return `null` in the results array: ```php new RequestConfig('GET', 'https://api.example.com/optional', - throwOnError: false, + throwOnExhausted: false, ) ``` @@ -134,7 +134,7 @@ new RequestConfig('GET', 'https://api.example.com/users', ) ``` -Throw `InvalidResponseException` from the parser to reject a semantically invalid 2xx response and trigger a retry on the same machinery (counts against `maxRetries`, fires `onRetry`, and on exhaustion fires `onExhausted` plus rethrows if `throwOnError: true`): +Throw `InvalidResponseException` from the parser to reject a semantically invalid 2xx response and trigger a retry on the same machinery (counts against `maxRetries`, fires `onRetry`, and on exhaustion fires `onExhausted` plus rethrows if `throwOnExhausted: true`): ```php use Shoxcie\BatchHttpClient\InvalidResponseException; @@ -176,7 +176,7 @@ $client = new BatchHttpClient($httpClient); | `url` | `string` | *(required)* | Request URL | | `options` | `array` | `[]` | Symfony HttpClient [options](https://symfony.com/doc/current/http_client.html#configuration) | | `retryOptions` | `array\|Closure` | `[]` | Options merged on retry, or Closure receiving `(int $attempt, Throwable $e)` | -| `throwOnError` | `bool` | `true` | Rethrow exception after retries exhausted | +| `throwOnExhausted` | `bool` | `true` | Rethrow exception after retries exhausted | | `decodeJson` | `bool` | `true` | Decode response as JSON | | `maxRetries` | `int` | `0` | Maximum retry attempts | | `retryOnTransportException` | `bool` | `true` | Retry on transport errors (timeouts, DNS) | diff --git a/example.php b/example.php index 040e157..68a749d 100644 --- a/example.php +++ b/example.php @@ -131,14 +131,14 @@ function logAbort(string $key, ResponseInterface $response, Throwable $e): void 'Request 1' => new RequestConfig('GET', '', options: ['base_uri' => 'https://httpbin.org/delay/7', 'timeout' => 5, 'max_duration' => 5], retryOptions: ['base_uri' => 'https://httpbin.org/get', 'timeout' => 1, 'max_duration' => 1], - throwOnError: false, + throwOnExhausted: false, decodeJson: true, maxRetries: 1 ), 'Request 2' => new RequestConfig('GET', '', options: ['base_uri' => 'https://httpbin.org/delay/7', 'timeout' => 1, 'max_duration' => 1], retryOptions: ['base_uri' => 'https://httpbin.org/delay/10', 'timeout' => 2, 'max_duration' => 2], - throwOnError: false, + throwOnExhausted: false, decodeJson: true, maxRetries: 1 ), diff --git a/src/BatchHttpClient.php b/src/BatchHttpClient.php index 61f2cee..f8cdec6 100644 --- a/src/BatchHttpClient.php +++ b/src/BatchHttpClient.php @@ -113,7 +113,7 @@ public function onAbort(Closure $closure): static * @return array * * @throws InvalidArgumentException if `retryOptions` contains the reserved `user_data` key - * @throws TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException if a `throwOnError` request fails after exhausting retries + * @throws TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException if a `throwOnExhausted` request fails after exhausting retries */ public function fetch(): array { @@ -204,7 +204,7 @@ private function handleRetryableException(ResponseInterface $response, Transport ($this->onExhausted)($key, $response, $e); } - if ($config->throwOnError) { + if ($config->throwOnExhausted) { throw $e; } diff --git a/src/RequestConfig.php b/src/RequestConfig.php index 8c29aa6..7f28a12 100644 --- a/src/RequestConfig.php +++ b/src/RequestConfig.php @@ -17,7 +17,7 @@ public function __construct( public array $options = [], /** @var array|Closure(int, Throwable): array */ public array|Closure $retryOptions = [], - public bool $throwOnError = true, + public bool $throwOnExhausted = true, public bool $decodeJson = true, public int $maxRetries = 0, public bool $retryOnTransportException = true, diff --git a/tests/Unit/BatchHttpClientTest.php b/tests/Unit/BatchHttpClientTest.php index 37e4877..92bba57 100644 --- a/tests/Unit/BatchHttpClientTest.php +++ b/tests/Unit/BatchHttpClientTest.php @@ -133,9 +133,9 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'users' => new RequestConfig('GET', 'https://api.example.com/users', throwOnError: false), - 'failing' => new RequestConfig('GET', 'https://api.example.com/failing', throwOnError: false), - 'orders' => new RequestConfig('GET', 'https://api.example.com/orders', throwOnError: false), + 'users' => new RequestConfig('GET', 'https://api.example.com/users', throwOnExhausted: false), + 'failing' => new RequestConfig('GET', 'https://api.example.com/failing', throwOnExhausted: false), + 'orders' => new RequestConfig('GET', 'https://api.example.com/orders', throwOnExhausted: false), ]) ->fetch(); @@ -160,10 +160,10 @@ function (string $method, string $url): JsonMockResponse { ); $configs = [ - 'ok-1' => new RequestConfig('GET', 'https://api.example.com/ok-1', throwOnError: false), - 'not-found' => new RequestConfig('GET', 'https://api.example.com/not-found', throwOnError: false), - 'ok-2' => new RequestConfig('GET', 'https://api.example.com/ok-2', throwOnError: false), - 'error' => new RequestConfig('GET', 'https://api.example.com/error', throwOnError: false), + 'ok-1' => new RequestConfig('GET', 'https://api.example.com/ok-1', throwOnExhausted: false), + 'not-found' => new RequestConfig('GET', 'https://api.example.com/not-found', throwOnExhausted: false), + 'ok-2' => new RequestConfig('GET', 'https://api.example.com/ok-2', throwOnExhausted: false), + 'error' => new RequestConfig('GET', 'https://api.example.com/error', throwOnExhausted: false), ]; $results = new BatchHttpClient($mockClient) @@ -182,7 +182,7 @@ function (string $method, string $url): JsonMockResponse { new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false, maxRetries: 3), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false, maxRetries: 3), ]) ->fetch(); @@ -196,7 +196,7 @@ function (string $method, string $url): JsonMockResponse { new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) ->fetch(); @@ -242,8 +242,8 @@ function (string $method, string $url) use (&$callCounts): JsonMockResponse { new BatchHttpClient($mockClient) ->request([ - 'alpha' => new RequestConfig('GET', 'https://api.example.com/alpha', throwOnError: false, maxRetries: 2), - 'beta' => new RequestConfig('GET', 'https://api.example.com/beta', throwOnError: false, maxRetries: 4), + 'alpha' => new RequestConfig('GET', 'https://api.example.com/alpha', throwOnExhausted: false, maxRetries: 2), + 'beta' => new RequestConfig('GET', 'https://api.example.com/beta', throwOnExhausted: false, maxRetries: 4), ]) ->fetch(); @@ -252,7 +252,7 @@ function (string $method, string $url) use (&$callCounts): JsonMockResponse { }); }); -describe('throwOnError: true', function (): void { +describe('throwOnExhausted: true', function (): void { test('throws exception after retries exhausted', function (): void { $mockClient = new MockHttpClient( fn(): JsonMockResponse => new JsonMockResponse([], ['http_code' => 500]), @@ -288,7 +288,7 @@ function (string $method, string $url) use (&$callCounts): JsonMockResponse { test('cancels all in-flight requests on failure', function (): void {})->todo(); }); -describe('throwOnError: false', function (): void { +describe('throwOnExhausted: false', function (): void { test('failed request returns null in results', function (): void { $mockClient = new MockHttpClient( fn(): JsonMockResponse => new JsonMockResponse([], ['http_code' => 500]), @@ -296,7 +296,7 @@ function (string $method, string $url) use (&$callCounts): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) ->fetch(); @@ -310,7 +310,7 @@ function (string $method, string $url) use (&$callCounts): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) ->fetch(); @@ -330,9 +330,9 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'failing' => new RequestConfig('GET', 'https://api.example.com/failing', throwOnError: false), - 'ok-1' => new RequestConfig('GET', 'https://api.example.com/ok-1', throwOnError: false), - 'ok-2' => new RequestConfig('GET', 'https://api.example.com/ok-2', throwOnError: false), + 'failing' => new RequestConfig('GET', 'https://api.example.com/failing', throwOnExhausted: false), + 'ok-1' => new RequestConfig('GET', 'https://api.example.com/ok-1', throwOnExhausted: false), + 'ok-2' => new RequestConfig('GET', 'https://api.example.com/ok-2', throwOnExhausted: false), ]) ->fetch(); @@ -349,7 +349,7 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false, maxRetries: 2), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false, maxRetries: 2), ]) ->fetch(); @@ -366,7 +366,7 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) ->fetch(); @@ -380,14 +380,14 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) ->fetch(); expect($results['api'])->toBeNull(); }); - test('throws transport exception with throwOnError true', function (): void { + test('throws transport exception with throwOnExhausted true', function (): void { $mockClient = new MockHttpClient([ new MockResponse(info: ['error' => 'Host unreachable']), ]); @@ -408,7 +408,7 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false, maxRetries: 2), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false, maxRetries: 2), ]) ->fetch(); @@ -425,7 +425,7 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false, maxRetries: 2, retryOnTransportException: true), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false, maxRetries: 2, retryOnTransportException: true), ]) ->fetch(); @@ -440,7 +440,7 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false, maxRetries: 2, retryOnTransportException: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false, maxRetries: 2, retryOnTransportException: false), ]) ->fetch(); @@ -455,7 +455,7 @@ function (string $method, string $url): JsonMockResponse { $results = new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false, maxRetries: 2, retryOnTransportException: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false, maxRetries: 2, retryOnTransportException: false), ]) ->fetch(); @@ -540,7 +540,7 @@ function () use (&$callCount): JsonMockResponse { new BatchHttpClient($mockClient) ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnError: false), + 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) ->onExhausted(function (string $key, ResponseInterface $response, \Throwable $e) use (&$captured): void { $captured[] = ['key' => $key, 'response' => $response, 'exception' => $e]; @@ -553,7 +553,7 @@ function () use (&$callCount): JsonMockResponse { ->and($captured[0]['exception'])->toBeInstanceOf(\Throwable::class); }); - test('onExhausted is invoked exactly once when throwOnError rethrows', function (): void { + test('onExhausted is invoked exactly once when throwOnExhausted rethrows', function (): void { $calls = 0; $mockClient = new MockHttpClient( fn(): JsonMockResponse => new JsonMockResponse([], ['http_code' => 500]), @@ -1087,7 +1087,7 @@ function () use (&$callCount): JsonMockResponse { ->and($retryCalls[0]['exception'])->toBeInstanceOf(InvalidResponseException::class); }); - test('exhausting retries with throwOnError false fires onExhausted and stores null', function (): void { + test('exhausting retries with throwOnExhausted false fires onExhausted and stores null', function (): void { $exhaustedCalls = []; $mockClient = new MockHttpClient( fn(): JsonMockResponse => new JsonMockResponse(['ok' => true]), @@ -1098,7 +1098,7 @@ function () use (&$callCount): JsonMockResponse { 'api' => new RequestConfig( 'GET', 'https://api.example.com/api', - throwOnError: false, + throwOnExhausted: false, maxRetries: 2, parseResponse: function (): mixed { throw new InvalidResponseException('always invalid'); @@ -1117,7 +1117,7 @@ function () use (&$callCount): JsonMockResponse { ->and($exhaustedCalls[0]['exception'])->toBeInstanceOf(InvalidResponseException::class); }); - test('exhausting retries with throwOnError true rethrows InvalidResponseException without firing onAbort', function (): void { + test('exhausting retries with throwOnExhausted true rethrows InvalidResponseException without firing onAbort', function (): void { $abortCalls = 0; $exhaustedCalls = 0; $mockClient = new MockHttpClient( @@ -1193,7 +1193,7 @@ function () use (&$callCount): JsonMockResponse { 'api' => new RequestConfig( 'GET', 'https://api.example.com/api', - throwOnError: false, + throwOnExhausted: false, parseResponse: function () use (&$parserCalls): mixed { ++$parserCalls; diff --git a/upgrade/4.0.md b/upgrade/4.0.md index fa7a641..d212e04 100644 --- a/upgrade/4.0.md +++ b/upgrade/4.0.md @@ -1,6 +1,6 @@ # Upgrading to 4.0 -Breaking callback signature change. +Two breaking changes — a callback signature update and a property rename. ## 1. `onSuccess()` callback receives the parsed result @@ -34,12 +34,37 @@ new BatchHttpClient() If a `RequestConfig` configured a `parseResponse` closure, `$result` is its return value (i.e. the same value `fetch()` returns in `$results[$key]`). With no `parseResponse`, `$result` is `array` (when `decodeJson: true`, the default) or `string` (when `decodeJson: false`). +## 2. `RequestConfig::$throwOnError` renamed to `$throwOnExhausted` + +The property only ever governed the post-retry-exhausted path — transient errors that retry-then-succeed never threw. The old name suggested broader semantics than the code delivered. The new name describes what actually happens and pairs with the existing `onExhausted` callback that fires under the same condition. + +Default (`true`), type (`bool`), and behavior are unchanged. This is a pure rename. + +| Before | After | +|---|---| +| `new RequestConfig(..., throwOnError: false)` | `new RequestConfig(..., throwOnExhausted: false)` | + +```php +// before +new RequestConfig('GET', 'https://api.example.com/optional', + throwOnError: false, + maxRetries: 3, +) + +// after +new RequestConfig('GET', 'https://api.example.com/optional', + throwOnExhausted: false, + maxRetries: 3, +) +``` + ## Finding callers before you upgrade -Grep for the call: +Grep for both: ``` grep -rn '->onSuccess(' src tests +grep -rn 'throwOnError' src tests ``` -Each match must add the `$result` parameter to its closure signature. +For `onSuccess`, each match must add the `$result` parameter. For `throwOnError`, each match must rename to `throwOnExhausted`. From f1274f14b0e0267dd194f6d292ee6d6c64adcd27 Mon Sep 17 00:00:00 2001 From: Shoxcie Date: Wed, 29 Apr 2026 14:36:27 +0300 Subject: [PATCH 4/5] feat!: widen retry catch to Symfony ExceptionInterface --- CHANGELOG.md | 1 + CLAUDE.md | 15 ++-- README.md | 6 +- example.php | 7 +- src/BatchHttpClient.php | 20 ++--- src/RequestConfig.php | 4 +- tests/Unit/BatchHttpClientTest.php | 115 +++++++++++++++++++++++------ upgrade/4.0.md | 35 ++++++++- 8 files changed, 153 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 497fa0f..92633e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `BatchHttpClient::onSuccess()` callback signature is now `Closure(string $key, mixed $result, ResponseInterface $response): void` (previously `Closure(string $key, ResponseInterface $response): void`). The `$result` is the value stored in `$results[$key]` — i.e. post-`parseResponse` if one is configured. Not contravariant: existing closures must add the `$result` parameter. - `RequestConfig::$throwOnError` renamed to `RequestConfig::$throwOnExhausted`. The previous name suggested "throw on any error" but the property only governs the post-retry-exhausted path; the new name aligns with the existing `onExhausted` callback. No behavior change. +- The retry catch now covers `Symfony\Contracts\HttpClient\Exception\ExceptionInterface | InvalidResponseException` (previously `TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException`). Decoding errors (notably malformed JSON via `decodeJson: true`) and redirection-without-follow errors are now retried instead of routed to `onAbort`. `onRetry()` and `onExhausted()` callback signatures widen to match. **Behavior change**: a 200 OK with malformed JSON now retries up to `maxRetries` and exhausts to `onExhausted` (or rethrows `JsonException` per `throwOnExhausted`), rather than firing `onAbort` and cancelling the batch. See [upgrade/4.0.md](upgrade/4.0.md) for migration details. diff --git a/CLAUDE.md b/CLAUDE.md index bb466f4..120be0f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,8 +21,8 @@ $results = $client 'orders' => new RequestConfig('POST', 'https://api.example.com/orders', options: ['json' => $data]), ]) ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) { ... }) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { ... }) - ->onExhausted(function (string $key, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e) { ... }) + ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { ... }) + ->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) { ... }) ->onAbort(function (string $key, ResponseInterface $response, Throwable $e) { ... }) ->fetch(); ``` @@ -51,22 +51,23 @@ Fires all HTTP requests immediately (Symfony HttpClient is async by default). St - Uses `stream()` with `isFirst` / `isLast` chunk pattern (Symfony docs recommended approach) - `isFirst`: acknowledges status code via `$response->getStatusCode()` to prevent generator auto-throw - `isLast`: reads response body, runs `parseResponse` if configured, stores result -- Non-2xx, transport errors, and `InvalidResponseException` from `parseResponse` caught via `catch (TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException)` +- Inner `catch (ExceptionInterface | InvalidResponseException)` covers any Symfony HttpClient exception (non-2xx, transport, decoding/JSON, redirection) and `InvalidResponseException` from `parseResponse`; routes them to `handleRetryableException()` for retry-or-exhaust - Outer `try/catch (Throwable)` as safety net — cancels all, calls `onAbort`, rethrows - Breaks out of `stream()` foreach only when a retry is scheduled (to restart stream with updated pool) ### Callbacks - `onSuccess(Closure)` — called for each 2xx response after `parseResponse` (if any) returns: `(string $key, mixed $result, ResponseInterface $response)`. `$result` is the value stored in `$results[$key]` (post-`parseResponse` if configured). -- `onRetry(Closure)` — called when a retry fires: `(string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse)` -- `onExhausted(Closure)` — called when a single request exhausts all retries: `(string $key, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e)` -- `onAbort(Closure)` — called when an unexpected exception cancels the whole batch (broken JSON, throwing user callback, parser throwing something other than `InvalidResponseException`, etc.): `(string $key, ResponseInterface $response, Throwable $e)`. Skipped if the throw happened before any response was processed. +- `onRetry(Closure)` — called when a retry fires: `(string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse)` +- `onExhausted(Closure)` — called when a single request exhausts all retries: `(string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e)` +- `onAbort(Closure)` — called when an unexpected `Throwable` (not an `ExceptionInterface` or `InvalidResponseException`) cancels the whole batch — e.g. a `RuntimeException` from a user callback, or anything other than `InvalidResponseException` thrown from `parseResponse`: `(string $key, ResponseInterface $response, Throwable $e)`. Skipped if the throw happened before any response was processed. ### Retry behavior - Any non-2xx HTTP status triggers a retry (Symfony throws `HttpExceptionInterface` which is caught) - Transport exceptions (connection timeout, DNS failure) retry based on `retryOnTransportException` per request (configurable, default true) -- A `parseResponse` closure throwing `InvalidResponseException` triggers a retry on the same machinery (counts against `maxRetries`, fires `onRetry`/`onExhausted`) +- Decoding errors (malformed JSON via `decodeJson: true`, any `DecodingExceptionInterface`) trigger a retry +- A `parseResponse` closure throwing `InvalidResponseException` triggers a retry (counts against `maxRetries`, fires `onRetry`/`onExhausted`) - Retries fire immediately (no backoff delay) - Retry requests use `array_replace_recursive($options, $retryOptions)` for options - `retryOptions` can be a Closure receiving `(int $attempt, Throwable $e)` for dynamic retry configuration diff --git a/README.md b/README.md index 749d9a6..2df95a2 100644 --- a/README.md +++ b/README.md @@ -82,10 +82,10 @@ $results = $client ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) { // called for each 2xx response, after parseResponse if configured }) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { + ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { // called when a retry fires }) - ->onExhausted(function (string $key, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e) { + ->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) { // called when a single request exhausts all retries }) ->onAbort(function (string $key, ResponseInterface $response, Throwable $e) { @@ -134,7 +134,7 @@ new RequestConfig('GET', 'https://api.example.com/users', ) ``` -Throw `InvalidResponseException` from the parser to reject a semantically invalid 2xx response and trigger a retry on the same machinery (counts against `maxRetries`, fires `onRetry`, and on exhaustion fires `onExhausted` plus rethrows if `throwOnExhausted: true`): +Throw `InvalidResponseException` from the parser to reject a semantically invalid 2xx response and trigger a retry (counts against `maxRetries`, fires `onRetry`, and on exhaustion fires `onExhausted` plus rethrows if `throwOnExhausted: true`): ```php use Shoxcie\BatchHttpClient\InvalidResponseException; diff --git a/example.php b/example.php index 68a749d..16051dd 100644 --- a/example.php +++ b/example.php @@ -7,8 +7,7 @@ use Shoxcie\BatchHttpClient\BatchHttpClient; use Shoxcie\BatchHttpClient\InvalidResponseException; use Shoxcie\BatchHttpClient\RequestConfig; -use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface; -use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; +use Symfony\Contracts\HttpClient\Exception\ExceptionInterface; use Symfony\Contracts\HttpClient\ResponseInterface; use function Shoxcie\BatchHttpClient\{get_content, get_headers, get_status_code, get_total_time, get_url}; @@ -73,7 +72,7 @@ function logSuccess(string $key, mixed $result, ResponseInterface $response): vo ); } -function logRetry(string $key, int $attempt, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void +function logRetry(string $key, int $attempt, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { simpleLog( prefix: 'RETRY', @@ -88,7 +87,7 @@ function logRetry(string $key, int $attempt, ResponseInterface $response, Transp ); } -function logExhausted(string $key, ResponseInterface $response, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e): void +function logExhausted(string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e): void { simpleLog( prefix: 'EXHAUSTED', diff --git a/src/BatchHttpClient.php b/src/BatchHttpClient.php index f8cdec6..9460dbb 100644 --- a/src/BatchHttpClient.php +++ b/src/BatchHttpClient.php @@ -7,7 +7,7 @@ use Closure; use InvalidArgumentException; use Symfony\Component\HttpClient\HttpClient; -use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface; +use Symfony\Contracts\HttpClient\Exception\ExceptionInterface; use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; use Symfony\Contracts\HttpClient\HttpClientInterface; use Symfony\Contracts\HttpClient\ResponseInterface; @@ -32,10 +32,10 @@ final class BatchHttpClient /** @var null|Closure(string, mixed, ResponseInterface): void */ private ?Closure $onSuccess = null; - /** @var null|Closure(string, int, ResponseInterface, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException, ResponseInterface): void */ + /** @var null|Closure(string, int, ResponseInterface, ExceptionInterface|InvalidResponseException, ResponseInterface): void */ private ?Closure $onRetry = null; - /** @var null|Closure(string, ResponseInterface, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException): void */ + /** @var null|Closure(string, ResponseInterface, ExceptionInterface|InvalidResponseException): void */ private ?Closure $onExhausted = null; /** @var null|Closure(string, ResponseInterface, Throwable): void */ @@ -85,7 +85,7 @@ public function onSuccess(Closure $closure): static return $this; } - /** @param Closure(string, int, ResponseInterface, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException, ResponseInterface): void $closure */ + /** @param Closure(string, int, ResponseInterface, ExceptionInterface|InvalidResponseException, ResponseInterface): void $closure */ public function onRetry(Closure $closure): static { $this->onRetry = $closure; @@ -93,7 +93,7 @@ public function onRetry(Closure $closure): static return $this; } - /** @param Closure(string, ResponseInterface, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException): void $closure */ + /** @param Closure(string, ResponseInterface, ExceptionInterface|InvalidResponseException): void $closure */ public function onExhausted(Closure $closure): static { $this->onExhausted = $closure; @@ -113,7 +113,7 @@ public function onAbort(Closure $closure): static * @return array * * @throws InvalidArgumentException if `retryOptions` contains the reserved `user_data` key - * @throws TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException if a `throwOnExhausted` request fails after exhausting retries + * @throws ExceptionInterface|InvalidResponseException if a `throwOnExhausted` request fails after exhausting retries */ public function fetch(): array { @@ -146,7 +146,7 @@ public function fetch(): array unset($this->responses[$key]); } - } catch (TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException $e) { + } catch (ExceptionInterface | InvalidResponseException $e) { if ($this->handleRetryableException($response, $e)) { break; } @@ -154,7 +154,7 @@ public function fetch(): array } } - } catch (TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException $e) { + } catch (ExceptionInterface | InvalidResponseException $e) { $this->cancelAll(); throw $e; @@ -174,7 +174,7 @@ public function fetch(): array return $this->results; } - private function handleRetryableException(ResponseInterface $response, TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException $e): bool + private function handleRetryableException(ResponseInterface $response, ExceptionInterface | InvalidResponseException $e): bool { $isTransportException = $e instanceof TransportExceptionInterface; @@ -222,7 +222,7 @@ private function getKey(ResponseInterface $response): string /** * @throws InvalidArgumentException if `$config->retryOptions` contains the reserved `user_data` key */ - private function retry(string $key, RequestConfig $config, TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException $e): ResponseInterface + private function retry(string $key, RequestConfig $config, ExceptionInterface | InvalidResponseException $e): ResponseInterface { ++$this->retriesCount[$key]; diff --git a/src/RequestConfig.php b/src/RequestConfig.php index 7f28a12..c42f1ed 100644 --- a/src/RequestConfig.php +++ b/src/RequestConfig.php @@ -5,8 +5,8 @@ namespace Shoxcie\BatchHttpClient; use Closure; +use Symfony\Contracts\HttpClient\Exception\ExceptionInterface; use Symfony\Contracts\HttpClient\ResponseInterface; -use Throwable; final readonly class RequestConfig { @@ -15,7 +15,7 @@ public function __construct( public string $url, /** @var array */ public array $options = [], - /** @var array|Closure(int, Throwable): array */ + /** @var array|Closure(int, ExceptionInterface|InvalidResponseException): array */ public array|Closure $retryOptions = [], public bool $throwOnExhausted = true, public bool $decodeJson = true, diff --git a/tests/Unit/BatchHttpClientTest.php b/tests/Unit/BatchHttpClientTest.php index 92bba57..deadd40 100644 --- a/tests/Unit/BatchHttpClientTest.php +++ b/tests/Unit/BatchHttpClientTest.php @@ -10,9 +10,8 @@ use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\JsonMockResponse; use Symfony\Component\HttpClient\Response\MockResponse; +use Symfony\Contracts\HttpClient\Exception\DecodingExceptionInterface; use Symfony\Contracts\HttpClient\Exception\ExceptionInterface; -use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface; -use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; use Symfony\Contracts\HttpClient\ResponseInterface; describe('successful batch requests (2xx)', function (): void { @@ -577,7 +576,7 @@ function () use (&$callCount): JsonMockResponse { $thrown = null; $mockClient = new MockHttpClient([ - new MockResponse('not json', ['response_headers' => ['content-type' => 'application/json']]), + new JsonMockResponse(['ok' => true]), ]); try { @@ -585,19 +584,22 @@ function () use (&$callCount): JsonMockResponse { ->request([ 'api' => new RequestConfig('GET', 'https://api.example.com/api'), ]) + ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { + throw new \RuntimeException('boom'); + }) ->onAbort(function (string $key, ResponseInterface $response, \Throwable $e) use (&$captured): void { $captured[] = ['key' => $key, 'response' => $response, 'exception' => $e]; }) ->fetch(); - } catch (\JsonException $e) { + } catch (\RuntimeException $e) { $thrown = $e; } - expect($thrown)->toBeInstanceOf(\JsonException::class) + expect($thrown)->toBeInstanceOf(\RuntimeException::class) ->and($captured)->toHaveCount(1) ->and($captured[0]['key'])->toBe('api') ->and($captured[0]['response'])->toBeInstanceOf(ResponseInterface::class) - ->and($captured[0]['exception'])->toBeInstanceOf(\JsonException::class); + ->and($captured[0]['exception'])->toBeInstanceOf(\RuntimeException::class); }); }); @@ -899,20 +901,6 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & }); describe('safety-net catch', function (): void { - test('rethrows JsonException on broken JSON with decodeJson true', function (): void { - $mockClient = new MockHttpClient([ - new MockResponse('not json', ['response_headers' => ['content-type' => 'application/json']]), - ]); - - expect( - fn(): array => new BatchHttpClient($mockClient) - ->request([ - 'api' => new RequestConfig('GET', 'https://api.example.com/api'), - ]) - ->fetch(), - )->toThrow(\JsonException::class); - }); - test('rethrows exception thrown from onSuccess callback', function (): void { $mockClient = new MockHttpClient([ new JsonMockResponse(['ok' => true]), @@ -954,6 +942,91 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & }); }); +describe('decoding errors', function (): void { + test('rethrows JsonException after exhausting retries (default maxRetries 0)', function (): void { + $mockClient = new MockHttpClient([ + new MockResponse('not json', ['response_headers' => ['content-type' => 'application/json']]), + ]); + + expect( + fn(): array => new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig('GET', 'https://api.example.com/api'), + ]) + ->fetch(), + )->toThrow(\JsonException::class); + }); + + test('decoding error triggers retry', function (): void { + $callCount = 0; + + $mockClient = new MockHttpClient( + function () use (&$callCount): MockResponse { + ++$callCount; + + return $callCount === 1 + ? new MockResponse('not json', ['response_headers' => ['content-type' => 'application/json']]) + : new JsonMockResponse(['ok' => true]); + }, + ); + + $retryCalls = []; + + $results = new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig( + 'GET', + 'https://api.example.com/api', + maxRetries: 1, + ), + ]) + ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { + $retryCalls[] = ['key' => $key, 'attempt' => $attempt, 'exception' => $e]; + }) + ->fetch(); + + expect($mockClient->getRequestsCount())->toBe(2) + ->and($results['api'])->toBe(['ok' => true]) + ->and($retryCalls)->toHaveCount(1) + ->and($retryCalls[0]['key'])->toBe('api') + ->and($retryCalls[0]['attempt'])->toBe(1) + ->and($retryCalls[0]['exception'])->toBeInstanceOf(DecodingExceptionInterface::class); + }); + + test('exhausting retries on decoding error fires onExhausted and stores null', function (): void { + $mockClient = new MockHttpClient( + fn(): MockResponse => new MockResponse('not json', ['response_headers' => ['content-type' => 'application/json']]), + ); + + $exhaustedCalls = []; + $abortCalls = []; + + $results = new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig( + 'GET', + 'https://api.example.com/api', + throwOnExhausted: false, + maxRetries: 2, + ), + ]) + ->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) use (&$exhaustedCalls): void { + $exhaustedCalls[] = ['key' => $key, 'exception' => $e]; + }) + ->onAbort(function (string $key, ResponseInterface $response, \Throwable $e) use (&$abortCalls): void { + $abortCalls[] = ['key' => $key, 'exception' => $e]; + }) + ->fetch(); + + expect($results['api'])->toBeNull() + ->and($mockClient->getRequestsCount())->toBe(3) + ->and($exhaustedCalls)->toHaveCount(1) + ->and($exhaustedCalls[0]['key'])->toBe('api') + ->and($exhaustedCalls[0]['exception'])->toBeInstanceOf(DecodingExceptionInterface::class) + ->and($abortCalls)->toBe([]); + }); +}); + describe('parseResponse', function (): void { test('return value replaces result in $results', function (): void { $mockClient = new MockHttpClient([ @@ -1074,7 +1147,7 @@ function () use (&$callCount): JsonMockResponse { }, ), ]) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { + ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { $retryCalls[] = ['key' => $key, 'attempt' => $attempt, 'exception' => $e]; }) ->fetch(); diff --git a/upgrade/4.0.md b/upgrade/4.0.md index d212e04..ba69975 100644 --- a/upgrade/4.0.md +++ b/upgrade/4.0.md @@ -1,6 +1,6 @@ # Upgrading to 4.0 -Two breaking changes — a callback signature update and a property rename. +Three breaking changes — a callback signature update, a property rename, and a widened retry catch. ## 1. `onSuccess()` callback receives the parsed result @@ -58,13 +58,42 @@ new RequestConfig('GET', 'https://api.example.com/optional', ) ``` +## 3. Retry catch widened to `ExceptionInterface` + +The retry catch was previously `TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException`. It is now `Symfony\Contracts\HttpClient\Exception\ExceptionInterface | InvalidResponseException`. This puts decoding errors (notably malformed JSON via `decodeJson: true`, anything implementing `DecodingExceptionInterface`) and redirection-without-follow errors on the same retry rails as transport and HTTP errors. + +The previous narrow catch left a hole: a 200 OK with malformed JSON killed the whole batch via `onAbort` with no retries, while a transient transport error on the same request would have retried gracefully. That asymmetry is the bug this fixes. The `onRetry` and `onExhausted` callback signatures widen accordingly. `onAbort` is unchanged — it still catches `Throwable` for genuinely unexpected exceptions (e.g., `RuntimeException` from a user callback) that aren't `ExceptionInterface` or `InvalidResponseException`. + +| Callback | Before | After | +|---|---|---| +| `onRetry`'s `$e` | `TransportExceptionInterface\|HttpExceptionInterface\|InvalidResponseException` | `ExceptionInterface\|InvalidResponseException` | +| `onExhausted`'s `$e` | `TransportExceptionInterface\|HttpExceptionInterface\|InvalidResponseException` | `ExceptionInterface\|InvalidResponseException` | +| `onAbort`'s `$e` | `Throwable` | `Throwable` (unchanged) | + +```php +// before +->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { + log_retry($key, $e); +}) + +// after +->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { + log_retry($key, $e); +}) +``` + +Closures that previously typed `$e` as just `\Throwable` or were untyped need no change — the new union is contravariant for them. + +**Behavior change**: a 200 OK with malformed JSON now retries up to `maxRetries` and exhausts to `onExhausted` (or rethrows `JsonException` per `throwOnExhausted`), instead of firing `onAbort`. If you want the old "abort the batch on this exception" behavior for some specific case, throw a non-`ExceptionInterface` `Throwable` (e.g., `\RuntimeException`) from your `parseResponse` — `onAbort`'s outer catch is still `Throwable`, so it grabs anything outside the retry union. + ## Finding callers before you upgrade -Grep for both: +Grep for all three: ``` grep -rn '->onSuccess(' src tests grep -rn 'throwOnError' src tests +grep -rn 'TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException' src tests ``` -For `onSuccess`, each match must add the `$result` parameter. For `throwOnError`, each match must rename to `throwOnExhausted`. +For `onSuccess`, each match must add the `$result` parameter. For `throwOnError`, each match must rename to `throwOnExhausted`. For the retry-catch union typehint, each match must collapse to `ExceptionInterface|InvalidResponseException` (and import `Symfony\Contracts\HttpClient\Exception\ExceptionInterface` if needed). From cc83d7639cde3bad137921cc4aa3f7254dc2796c Mon Sep 17 00:00:00 2001 From: Shoxcie Date: Wed, 29 Apr 2026 15:50:21 +0300 Subject: [PATCH 5/5] feat!: add $retries to all observability callbacks --- CHANGELOG.md | 4 +- CLAUDE.md | 20 ++-- README.md | 21 ++-- example.php | 19 ++-- src/BatchHttpClient.php | 18 +-- tests/Unit/BatchHttpClientTest.php | 170 ++++++++++++++++++++++------- upgrade/4.0.md | 77 +++++++++++-- 7 files changed, 243 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92633e0..2f133f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- `BatchHttpClient::onSuccess()` callback signature is now `Closure(string $key, mixed $result, ResponseInterface $response): void` (previously `Closure(string $key, ResponseInterface $response): void`). The `$result` is the value stored in `$results[$key]` — i.e. post-`parseResponse` if one is configured. Not contravariant: existing closures must add the `$result` parameter. +- `BatchHttpClient::onSuccess()` callback signature is now `Closure(string $key, int $retries, mixed $result, ResponseInterface $response): void` (previously `Closure(string $key, ResponseInterface $response): void`). `$retries` is the number of retries that happened before the success (0 for first-attempt, N for success after N retries). `$result` is the value stored in `$results[$key]` — i.e. post-`parseResponse` if one is configured. Not contravariant: existing closures must add the new positional parameters. +- `BatchHttpClient::onExhausted()` callback signature is now `Closure(string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e): void` (previously without `$retries`). `$retries` equals `maxRetries` on normal exhaustion and is less than `maxRetries` when a transport error short-circuits with `retryOnTransportException: false`. Not contravariant: existing closures must add the `$retries` parameter. +- `BatchHttpClient::onAbort()` callback signature is now `Closure(string $key, int $retries, ResponseInterface $response, Throwable $e): void` (previously without `$retries`). `$retries` is the retry count for the request being processed when the unexpected abort fired. Not contravariant: existing closures must add the `$retries` parameter. With this change, all four callbacks share the `(key, retries, ...)` prefix. - `RequestConfig::$throwOnError` renamed to `RequestConfig::$throwOnExhausted`. The previous name suggested "throw on any error" but the property only governs the post-retry-exhausted path; the new name aligns with the existing `onExhausted` callback. No behavior change. - The retry catch now covers `Symfony\Contracts\HttpClient\Exception\ExceptionInterface | InvalidResponseException` (previously `TransportExceptionInterface | HttpExceptionInterface | InvalidResponseException`). Decoding errors (notably malformed JSON via `decodeJson: true`) and redirection-without-follow errors are now retried instead of routed to `onAbort`. `onRetry()` and `onExhausted()` callback signatures widen to match. **Behavior change**: a 200 OK with malformed JSON now retries up to `maxRetries` and exhausts to `onExhausted` (or rethrows `JsonException` per `throwOnExhausted`), rather than firing `onAbort` and cancelling the batch. diff --git a/CLAUDE.md b/CLAUDE.md index 120be0f..eeda6dc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,10 +20,10 @@ $results = $client 'users' => new RequestConfig('GET', 'https://api.example.com/users', maxRetries: 3), 'orders' => new RequestConfig('POST', 'https://api.example.com/orders', options: ['json' => $data]), ]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) { ... }) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { ... }) - ->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) { ... }) - ->onAbort(function (string $key, ResponseInterface $response, Throwable $e) { ... }) + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response) { ... }) + ->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { ... }) + ->onExhausted(function (string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) { ... }) + ->onAbort(function (string $key, int $retries, ResponseInterface $response, Throwable $e) { ... }) ->fetch(); ``` @@ -34,7 +34,7 @@ $results = $client - `method` (string) - `url` (string) - `options` (array, default `[]`) — standard Symfony HttpClient options (timeout, max_duration, headers, etc.) - - `retryOptions` (array|Closure, default `[]`) — merged onto options for retries via `array_replace_recursive($options, $retryOptions)`. If Closure: receives `(int $attempt, Throwable $e)`, must return options array. + - `retryOptions` (array|Closure, default `[]`) — merged onto options for retries via `array_replace_recursive($options, $retryOptions)`. If Closure: receives `(int $retries, ExceptionInterface|InvalidResponseException $e)`, must return options array. - `throwOnExhausted` (bool, default `true`) — if true and request exhausts all retries, rethrow last exception and cancel all in-flight requests - `decodeJson` (bool, default `true`) — `true`: `toArray()`, `false`: `getContent()` - `maxRetries` (int, default `0`) — max retry count @@ -57,10 +57,10 @@ Fires all HTTP requests immediately (Symfony HttpClient is async by default). St ### Callbacks -- `onSuccess(Closure)` — called for each 2xx response after `parseResponse` (if any) returns: `(string $key, mixed $result, ResponseInterface $response)`. `$result` is the value stored in `$results[$key]` (post-`parseResponse` if configured). -- `onRetry(Closure)` — called when a retry fires: `(string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse)` -- `onExhausted(Closure)` — called when a single request exhausts all retries: `(string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e)` -- `onAbort(Closure)` — called when an unexpected `Throwable` (not an `ExceptionInterface` or `InvalidResponseException`) cancels the whole batch — e.g. a `RuntimeException` from a user callback, or anything other than `InvalidResponseException` thrown from `parseResponse`: `(string $key, ResponseInterface $response, Throwable $e)`. Skipped if the throw happened before any response was processed. +- `onSuccess(Closure)` — called for each 2xx response after `parseResponse` (if any) returns: `(string $key, int $retries, mixed $result, ResponseInterface $response)`. `$retries` is the number of retries before success (0 = first-attempt). `$result` is the value stored in `$results[$key]` (post-`parseResponse` if configured). +- `onRetry(Closure)` — called when a retry fires: `(string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse)`. `$retries` is the retry count after this retry fired (always ≥ 1). +- `onExhausted(Closure)` — called when a single request exhausts all retries: `(string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e)`. `$retries` equals `maxRetries` on normal exhaustion; less when a transport error short-circuits with `retryOnTransportException: false`. +- `onAbort(Closure)` — called when an unexpected `Throwable` (not an `ExceptionInterface` or `InvalidResponseException`) cancels the whole batch — e.g. a `RuntimeException` from a user callback, or anything other than `InvalidResponseException` thrown from `parseResponse`: `(string $key, int $retries, ResponseInterface $response, Throwable $e)`. `$retries` is the retry count for the request being processed when the abort fired. Skipped if the throw happened before any response was processed. ### Retry behavior @@ -70,7 +70,7 @@ Fires all HTTP requests immediately (Symfony HttpClient is async by default). St - A `parseResponse` closure throwing `InvalidResponseException` triggers a retry (counts against `maxRetries`, fires `onRetry`/`onExhausted`) - Retries fire immediately (no backoff delay) - Retry requests use `array_replace_recursive($options, $retryOptions)` for options -- `retryOptions` can be a Closure receiving `(int $attempt, Throwable $e)` for dynamic retry configuration +- `retryOptions` can be a Closure receiving `(int $retries, ExceptionInterface|InvalidResponseException $e)` for dynamic retry configuration - Each request retries independently without blocking others - `$response->cancel()` called on transport errors before retry to free the broken socket diff --git a/README.md b/README.md index 2df95a2..3f42e48 100644 --- a/README.md +++ b/README.md @@ -66,8 +66,8 @@ Or dynamically with a Closure: ```php new RequestConfig('GET', 'https://api.example.com/resource', maxRetries: 3, - retryOptions: function (int $attempt, Throwable $e): array { - return ['timeout' => 10 * $attempt]; + retryOptions: function (int $retries, ExceptionInterface|InvalidResponseException $e): array { + return ['timeout' => 10 * $retries]; }, ) ``` @@ -79,17 +79,20 @@ Retry options are merged onto the original options via `array_replace_recursive( ```php $results = $client ->request([...]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) { + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response) { // called for each 2xx response, after parseResponse if configured + // $retries = 0 on first-attempt success, N on success after N retries }) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { - // called when a retry fires + ->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) { + // called when a retry fires; $retries is the retry count after this retry fired (always >= 1) }) - ->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) { + ->onExhausted(function (string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) { // called when a single request exhausts all retries + // $retries equals maxRetries normally; less when a transport error short-circuits }) - ->onAbort(function (string $key, ResponseInterface $response, Throwable $e) { - // called when an unexpected exception (broken JSON, throwing callback, ...) cancels the whole batch + ->onAbort(function (string $key, int $retries, ResponseInterface $response, Throwable $e) { + // called when an unexpected exception (e.g. throwing user callback) cancels the whole batch + // $retries is the retry count for the request being processed when the abort fired }) ->fetch(); ``` @@ -175,7 +178,7 @@ $client = new BatchHttpClient($httpClient); | `method` | `string` | *(required)* | HTTP method | | `url` | `string` | *(required)* | Request URL | | `options` | `array` | `[]` | Symfony HttpClient [options](https://symfony.com/doc/current/http_client.html#configuration) | -| `retryOptions` | `array\|Closure` | `[]` | Options merged on retry, or Closure receiving `(int $attempt, Throwable $e)` | +| `retryOptions` | `array\|Closure` | `[]` | Options merged on retry, or Closure receiving `(int $retries, ExceptionInterface\|InvalidResponseException $e)` | | `throwOnExhausted` | `bool` | `true` | Rethrow exception after retries exhausted | | `decodeJson` | `bool` | `true` | Decode response as JSON | | `maxRetries` | `int` | `0` | Maximum retry attempts | diff --git a/example.php b/example.php index 16051dd..fc7d0dd 100644 --- a/example.php +++ b/example.php @@ -21,7 +21,7 @@ function simpleLog( string $url, float $duration, ?string $exceptionMessage = null, - ?int $attempt = null, + ?int $retries = null, ?int $statusCode = null, ?array $headers = null, ?string $body = null @@ -40,8 +40,8 @@ function simpleLog( $parts[] = 'Exception => ' . explode(' for "', $exceptionMessage)[0]; } - if (isset($attempt)) { - $parts[] = 'Attempt => ' . $attempt; + if (isset($retries)) { + $parts[] = 'Retries => ' . $retries; } if ($statusCode) { @@ -59,27 +59,28 @@ function simpleLog( echo implode(' | ', $parts) . PHP_EOL . PHP_EOL; } -function logSuccess(string $key, mixed $result, ResponseInterface $response): void +function logSuccess(string $key, int $retries, mixed $result, ResponseInterface $response): void { simpleLog( prefix: 'SUCCESS', key: $key, url: get_url($response), duration: get_total_time($response), + retries: $retries, statusCode: get_status_code($response), headers: get_headers($response), body: get_content($response), ); } -function logRetry(string $key, int $attempt, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void +function logRetry(string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { simpleLog( prefix: 'RETRY', key: $key, url: get_url($response), duration: get_total_time($response), - attempt: $attempt, + retries: $retries, exceptionMessage: $e->getMessage(), statusCode: get_status_code($response), headers: get_headers($response), @@ -87,13 +88,14 @@ function logRetry(string $key, int $attempt, ResponseInterface $response, Except ); } -function logExhausted(string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e): void +function logExhausted(string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e): void { simpleLog( prefix: 'EXHAUSTED', key: $key, url: get_url($response), duration: get_total_time($response), + retries: $retries, exceptionMessage: $e->getMessage(), statusCode: get_status_code($response), headers: get_headers($response), @@ -101,13 +103,14 @@ function logExhausted(string $key, ResponseInterface $response, ExceptionInterfa ); } -function logAbort(string $key, ResponseInterface $response, Throwable $e): void +function logAbort(string $key, int $retries, ResponseInterface $response, Throwable $e): void { simpleLog( prefix: 'ABORT', key: $key, url: get_url($response), duration: get_total_time($response), + retries: $retries, exceptionMessage: $e->getMessage(), statusCode: get_status_code($response), headers: get_headers($response), diff --git a/src/BatchHttpClient.php b/src/BatchHttpClient.php index 9460dbb..95626f1 100644 --- a/src/BatchHttpClient.php +++ b/src/BatchHttpClient.php @@ -29,16 +29,16 @@ final class BatchHttpClient /** @var array */ private array $results = []; - /** @var null|Closure(string, mixed, ResponseInterface): void */ + /** @var null|Closure(string, int, mixed, ResponseInterface): void */ private ?Closure $onSuccess = null; /** @var null|Closure(string, int, ResponseInterface, ExceptionInterface|InvalidResponseException, ResponseInterface): void */ private ?Closure $onRetry = null; - /** @var null|Closure(string, ResponseInterface, ExceptionInterface|InvalidResponseException): void */ + /** @var null|Closure(string, int, ResponseInterface, ExceptionInterface|InvalidResponseException): void */ private ?Closure $onExhausted = null; - /** @var null|Closure(string, ResponseInterface, Throwable): void */ + /** @var null|Closure(string, int, ResponseInterface, Throwable): void */ private ?Closure $onAbort = null; public function __construct( @@ -77,7 +77,7 @@ public function request(array $configs): static return $this; } - /** @param Closure(string, mixed, ResponseInterface): void $closure */ + /** @param Closure(string, int, mixed, ResponseInterface): void $closure */ public function onSuccess(Closure $closure): static { $this->onSuccess = $closure; @@ -93,7 +93,7 @@ public function onRetry(Closure $closure): static return $this; } - /** @param Closure(string, ResponseInterface, ExceptionInterface|InvalidResponseException): void $closure */ + /** @param Closure(string, int, ResponseInterface, ExceptionInterface|InvalidResponseException): void $closure */ public function onExhausted(Closure $closure): static { $this->onExhausted = $closure; @@ -101,7 +101,7 @@ public function onExhausted(Closure $closure): static return $this; } - /** @param Closure(string, ResponseInterface, Throwable): void $closure */ + /** @param Closure(string, int, ResponseInterface, Throwable): void $closure */ public function onAbort(Closure $closure): static { $this->onAbort = $closure; @@ -140,7 +140,7 @@ public function fetch(): array $this->results[$key] = $result; if ($this->onSuccess instanceof Closure) { - ($this->onSuccess)($key, $result, $response); + ($this->onSuccess)($key, $this->retriesCount[$key], $result, $response); } unset($this->responses[$key]); @@ -165,7 +165,7 @@ public function fetch(): array if (isset($response) && $this->onAbort instanceof Closure) { $key = $this->getKey($response); - ($this->onAbort)($key, $response, $e); + ($this->onAbort)($key, $this->retriesCount[$key], $response, $e); } throw $e; @@ -201,7 +201,7 @@ private function handleRetryableException(ResponseInterface $response, Exception } if ($this->onExhausted instanceof Closure) { - ($this->onExhausted)($key, $response, $e); + ($this->onExhausted)($key, $this->retriesCount[$key], $response, $e); } if ($config->throwOnExhausted) { diff --git a/tests/Unit/BatchHttpClientTest.php b/tests/Unit/BatchHttpClientTest.php index deadd40..678f58c 100644 --- a/tests/Unit/BatchHttpClientTest.php +++ b/tests/Unit/BatchHttpClientTest.php @@ -464,7 +464,7 @@ function (string $method, string $url): JsonMockResponse { }); describe('callbacks', function (): void { - test('onSuccess receives key, result, and response', function (): void { + test('onSuccess receives key, retries, result, and response', function (): void { $captured = []; $mockClient = new MockHttpClient([ @@ -477,21 +477,50 @@ function (string $method, string $url): JsonMockResponse { 'users' => new RequestConfig('GET', 'https://api.example.com/users'), 'orders' => new RequestConfig('GET', 'https://api.example.com/orders'), ]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) use (&$captured): void { - $captured[] = ['key' => $key, 'result' => $result, 'response' => $response]; + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response) use (&$captured): void { + $captured[] = ['key' => $key, 'retries' => $retries, 'result' => $result, 'response' => $response]; }) ->fetch(); expect($captured)->toHaveCount(2) ->and($captured[0]['key'])->toBe('users') + ->and($captured[0]['retries'])->toBe(0) ->and($captured[0]['result'])->toBe(['id' => 1]) ->and($captured[0]['response'])->toBeInstanceOf(ResponseInterface::class) ->and($captured[1]['key'])->toBe('orders') + ->and($captured[1]['retries'])->toBe(0) ->and($captured[1]['result'])->toBe(['id' => 2]) ->and($captured[1]['response'])->toBeInstanceOf(ResponseInterface::class); }); - test('onRetry receives key, attempt, failed response, exception, and retry response', function (): void { + test('onSuccess receives non-zero retries when success follows a retry', function (): void { + $callCount = 0; + + $mockClient = new MockHttpClient( + function () use (&$callCount): JsonMockResponse { + ++$callCount; + + return $callCount === 1 + ? new JsonMockResponse([], ['http_code' => 500]) + : new JsonMockResponse(['ok' => true]); + }, + ); + + $capturedRetries = null; + + new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig('GET', 'https://api.example.com/api', maxRetries: 1), + ]) + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response) use (&$capturedRetries): void { + $capturedRetries = $retries; + }) + ->fetch(); + + expect($capturedRetries)->toBe(1); + }); + + test('onRetry receives key, retries, failed response, exception, and retry response', function (): void { $captured = []; $callCount = 0; @@ -511,10 +540,10 @@ function () use (&$callCount): JsonMockResponse { ->request([ 'api' => new RequestConfig('GET', 'https://api.example.com/api', maxRetries: 2), ]) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface $e, ResponseInterface $retryResponse) use (&$captured): void { + ->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface $e, ResponseInterface $retryResponse) use (&$captured): void { $captured[] = [ 'key' => $key, - 'attempt' => $attempt, + 'retries' => $retries, 'failedResponse' => $failedResponse, 'exception' => $e, 'retryResponse' => $retryResponse, @@ -524,13 +553,13 @@ function () use (&$callCount): JsonMockResponse { expect($captured)->toHaveCount(1) ->and($captured[0]['key'])->toBe('api') - ->and($captured[0]['attempt'])->toBe(1) + ->and($captured[0]['retries'])->toBe(1) ->and($captured[0]['failedResponse'])->toBeInstanceOf(ResponseInterface::class) ->and($captured[0]['exception'])->toBeInstanceOf(ExceptionInterface::class) ->and($captured[0]['retryResponse'])->toBeInstanceOf(ResponseInterface::class); }); - test('onExhausted receives key, response, and exception', function (): void { + test('onExhausted receives key, retries, response, and exception', function (): void { $captured = []; $mockClient = new MockHttpClient([ @@ -541,19 +570,21 @@ function () use (&$callCount): JsonMockResponse { ->request([ 'api' => new RequestConfig('GET', 'https://api.example.com/api', throwOnExhausted: false), ]) - ->onExhausted(function (string $key, ResponseInterface $response, \Throwable $e) use (&$captured): void { - $captured[] = ['key' => $key, 'response' => $response, 'exception' => $e]; + ->onExhausted(function (string $key, int $retries, ResponseInterface $response, \Throwable $e) use (&$captured): void { + $captured[] = ['key' => $key, 'retries' => $retries, 'response' => $response, 'exception' => $e]; }) ->fetch(); expect($captured)->toHaveCount(1) ->and($captured[0]['key'])->toBe('api') + ->and($captured[0]['retries'])->toBe(0) ->and($captured[0]['response'])->toBeInstanceOf(ResponseInterface::class) ->and($captured[0]['exception'])->toBeInstanceOf(\Throwable::class); }); test('onExhausted is invoked exactly once when throwOnExhausted rethrows', function (): void { $calls = 0; + $capturedRetries = null; $mockClient = new MockHttpClient( fn(): JsonMockResponse => new JsonMockResponse([], ['http_code' => 500]), ); @@ -561,17 +592,44 @@ function () use (&$callCount): JsonMockResponse { try { new BatchHttpClient($mockClient) ->request(['api' => new RequestConfig('GET', 'https://api.example.com/api')]) - ->onExhausted(function () use (&$calls): void { + ->onExhausted(function (string $key, int $retries) use (&$calls, &$capturedRetries): void { ++$calls; + $capturedRetries = $retries; }) ->fetch(); } catch (ServerException) { } - expect($calls)->toBe(1); + expect($calls)->toBe(1) + ->and($capturedRetries)->toBe(0); }); - test('onAbort receives key, response, and exception on unexpected exception', function (): void { + test('onExhausted receives retries less than maxRetries when transport short-circuits', function (): void { + $capturedRetries = null; + + $mockClient = new MockHttpClient( + fn(): MockResponse => new MockResponse(info: ['error' => 'connection refused']), + ); + + new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig( + 'GET', + 'https://api.example.com/api', + throwOnExhausted: false, + maxRetries: 5, + retryOnTransportException: false, + ), + ]) + ->onExhausted(function (string $key, int $retries, ResponseInterface $response, $e) use (&$capturedRetries): void { + $capturedRetries = $retries; + }) + ->fetch(); + + expect($capturedRetries)->toBe(0); + }); + + test('onAbort receives key, retries, response, and exception on unexpected exception', function (): void { $captured = []; $thrown = null; @@ -584,11 +642,11 @@ function () use (&$callCount): JsonMockResponse { ->request([ 'api' => new RequestConfig('GET', 'https://api.example.com/api'), ]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response): void { throw new \RuntimeException('boom'); }) - ->onAbort(function (string $key, ResponseInterface $response, \Throwable $e) use (&$captured): void { - $captured[] = ['key' => $key, 'response' => $response, 'exception' => $e]; + ->onAbort(function (string $key, int $retries, ResponseInterface $response, \Throwable $e) use (&$captured): void { + $captured[] = ['key' => $key, 'retries' => $retries, 'response' => $response, 'exception' => $e]; }) ->fetch(); } catch (\RuntimeException $e) { @@ -598,10 +656,44 @@ function () use (&$callCount): JsonMockResponse { expect($thrown)->toBeInstanceOf(\RuntimeException::class) ->and($captured)->toHaveCount(1) ->and($captured[0]['key'])->toBe('api') + ->and($captured[0]['retries'])->toBe(0) ->and($captured[0]['response'])->toBeInstanceOf(ResponseInterface::class) ->and($captured[0]['exception'])->toBeInstanceOf(\RuntimeException::class); }); + test('onAbort receives non-zero retries when abort follows a retry', function (): void { + $callCount = 0; + + $mockClient = new MockHttpClient( + function () use (&$callCount): JsonMockResponse { + ++$callCount; + + return $callCount === 1 + ? new JsonMockResponse([], ['http_code' => 500]) + : new JsonMockResponse(['ok' => true]); + }, + ); + + $capturedRetries = null; + + try { + new BatchHttpClient($mockClient) + ->request([ + 'api' => new RequestConfig('GET', 'https://api.example.com/api', maxRetries: 1), + ]) + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response): void { + throw new \RuntimeException('boom'); + }) + ->onAbort(function (string $key, int $retries, ResponseInterface $response, \Throwable $e) use (&$capturedRetries): void { + $capturedRetries = $retries; + }) + ->fetch(); + } catch (\RuntimeException) { + } + + expect($capturedRetries)->toBe(1); + }); + }); describe('decodeJson', function (): void { @@ -778,8 +870,8 @@ function () use (&$callCount): JsonMockResponse { 'api' => new RequestConfig( 'GET', 'https://api.example.com/api', - retryOptions: function (int $attempt, \Throwable $e) use (&$captured): array { - $captured[] = ['attempt' => $attempt, 'exception' => $e]; + retryOptions: function (int $retries, \Throwable $e) use (&$captured): array { + $captured[] = ['retries' => $retries, 'exception' => $e]; return []; }, @@ -789,9 +881,9 @@ function () use (&$callCount): JsonMockResponse { ->fetch(); expect($captured)->toHaveCount(2) - ->and($captured[0]['attempt'])->toBe(1) + ->and($captured[0]['retries'])->toBe(1) ->and($captured[0]['exception'])->toBeInstanceOf(ExceptionInterface::class) - ->and($captured[1]['attempt'])->toBe(2) + ->and($captured[1]['retries'])->toBe(2) ->and($captured[1]['exception'])->toBeInstanceOf(ExceptionInterface::class); }); @@ -817,7 +909,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & 'api' => new RequestConfig( 'GET', 'https://api.example.com/api', - retryOptions: fn(int $attempt, \Throwable $e): array => ['headers' => ['X-Attempt' => '1']], + retryOptions: fn(int $retries, \Throwable $e): array => ['headers' => ['X-Attempt' => '1']], maxRetries: 1, ), ]) @@ -849,7 +941,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & 'api' => new RequestConfig( 'GET', 'https://api.example.com/api', - retryOptions: fn(int $attempt, \Throwable $e): array => ['headers' => ['X-Attempt' => (string) $attempt]], + retryOptions: fn(int $retries, \Throwable $e): array => ['headers' => ['X-Attempt' => (string) $retries]], maxRetries: 2, ), ]) @@ -891,7 +983,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & 'api' => new RequestConfig( 'GET', 'https://api.example.com/api', - retryOptions: fn(int $attempt, \Throwable $e): array => ['user_data' => 'nope'], + retryOptions: fn(int $retries, \Throwable $e): array => ['user_data' => 'nope'], maxRetries: 1, ), ]) @@ -911,7 +1003,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & ->request([ 'api' => new RequestConfig('GET', 'https://api.example.com/api'), ]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response): void { throw new \RuntimeException('boom'); }) ->fetch(), @@ -932,7 +1024,7 @@ function (string $method, string $url, array $options) use (&$capturedHeaders, & 'b' => new RequestConfig('GET', 'https://api.example.com/b', maxRetries: 3), 'c' => new RequestConfig('GET', 'https://api.example.com/c', maxRetries: 3), ]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response): void { throw new \RuntimeException('boom'); }) ->fetch(), @@ -980,8 +1072,8 @@ function () use (&$callCount): MockResponse { maxRetries: 1, ), ]) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { - $retryCalls[] = ['key' => $key, 'attempt' => $attempt, 'exception' => $e]; + ->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { + $retryCalls[] = ['key' => $key, 'retries' => $retries, 'exception' => $e]; }) ->fetch(); @@ -989,7 +1081,7 @@ function () use (&$callCount): MockResponse { ->and($results['api'])->toBe(['ok' => true]) ->and($retryCalls)->toHaveCount(1) ->and($retryCalls[0]['key'])->toBe('api') - ->and($retryCalls[0]['attempt'])->toBe(1) + ->and($retryCalls[0]['retries'])->toBe(1) ->and($retryCalls[0]['exception'])->toBeInstanceOf(DecodingExceptionInterface::class); }); @@ -1010,10 +1102,10 @@ function () use (&$callCount): MockResponse { maxRetries: 2, ), ]) - ->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) use (&$exhaustedCalls): void { - $exhaustedCalls[] = ['key' => $key, 'exception' => $e]; + ->onExhausted(function (string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e) use (&$exhaustedCalls): void { + $exhaustedCalls[] = ['key' => $key, 'retries' => $retries, 'exception' => $e]; }) - ->onAbort(function (string $key, ResponseInterface $response, \Throwable $e) use (&$abortCalls): void { + ->onAbort(function (string $key, int $retries, ResponseInterface $response, \Throwable $e) use (&$abortCalls): void { $abortCalls[] = ['key' => $key, 'exception' => $e]; }) ->fetch(); @@ -1022,6 +1114,7 @@ function () use (&$callCount): MockResponse { ->and($mockClient->getRequestsCount())->toBe(3) ->and($exhaustedCalls)->toHaveCount(1) ->and($exhaustedCalls[0]['key'])->toBe('api') + ->and($exhaustedCalls[0]['retries'])->toBe(2) ->and($exhaustedCalls[0]['exception'])->toBeInstanceOf(DecodingExceptionInterface::class) ->and($abortCalls)->toBe([]); }); @@ -1061,7 +1154,7 @@ function () use (&$callCount): MockResponse { parseResponse: fn(string $key, mixed $result, ResponseInterface $response): mixed => $result['data'], ), ]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response) use (&$captured): void { + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response) use (&$captured): void { $captured = $result; }) ->fetch(); @@ -1147,8 +1240,8 @@ function () use (&$callCount): JsonMockResponse { }, ), ]) - ->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { - $retryCalls[] = ['key' => $key, 'attempt' => $attempt, 'exception' => $e]; + ->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse) use (&$retryCalls): void { + $retryCalls[] = ['key' => $key, 'retries' => $retries, 'exception' => $e]; }) ->fetch(); @@ -1156,7 +1249,7 @@ function () use (&$callCount): JsonMockResponse { ->and($results['api'])->toBe(['attempt' => 2]) ->and($retryCalls)->toHaveCount(1) ->and($retryCalls[0]['key'])->toBe('api') - ->and($retryCalls[0]['attempt'])->toBe(1) + ->and($retryCalls[0]['retries'])->toBe(1) ->and($retryCalls[0]['exception'])->toBeInstanceOf(InvalidResponseException::class); }); @@ -1178,8 +1271,8 @@ function () use (&$callCount): JsonMockResponse { }, ), ]) - ->onExhausted(function (string $key, ResponseInterface $response, $e) use (&$exhaustedCalls): void { - $exhaustedCalls[] = ['key' => $key, 'exception' => $e]; + ->onExhausted(function (string $key, int $retries, ResponseInterface $response, $e) use (&$exhaustedCalls): void { + $exhaustedCalls[] = ['key' => $key, 'retries' => $retries, 'exception' => $e]; }) ->fetch(); @@ -1187,6 +1280,7 @@ function () use (&$callCount): JsonMockResponse { ->and($results['api'])->toBeNull() ->and($exhaustedCalls)->toHaveCount(1) ->and($exhaustedCalls[0]['key'])->toBe('api') + ->and($exhaustedCalls[0]['retries'])->toBe(2) ->and($exhaustedCalls[0]['exception'])->toBeInstanceOf(InvalidResponseException::class); }); @@ -1244,7 +1338,7 @@ function () use (&$callCount): JsonMockResponse { }, ), ]) - ->onAbort(function (string $key, ResponseInterface $response, \Throwable $e) use (&$aborted): void { + ->onAbort(function (string $key, int $retries, ResponseInterface $response, \Throwable $e) use (&$aborted): void { $aborted = ['key' => $key, 'exception' => $e]; }) ->fetch(); diff --git a/upgrade/4.0.md b/upgrade/4.0.md index ba69975..c032ef9 100644 --- a/upgrade/4.0.md +++ b/upgrade/4.0.md @@ -1,16 +1,16 @@ # Upgrading to 4.0 -Three breaking changes — a callback signature update, a property rename, and a widened retry catch. +Five breaking changes — three callback signature updates, a property rename, and a widened retry catch. -## 1. `onSuccess()` callback receives the parsed result +## 1. `onSuccess()` callback receives retry count and parsed result -`onSuccess` now gets the value already stored in the results array as a third positional argument, mirroring `parseResponse`. Callbacks no longer need to call `$response->toArray()` again to inspect the body, and they automatically see whatever `parseResponse` returned (if one is configured). +`onSuccess` gains two new positional arguments: `int $retries` (number of retries that happened before this success — `0` for first-attempt, `N` after `N` retries) and `mixed $result` (the value already stored in the results array, post-`parseResponse` if one is configured). Callbacks no longer need to call `$response->toArray()` again to inspect the body, and `$retries` makes "succeeded on retry" logging trivial. -This is **not** contravariant — every existing `onSuccess` closure must add the new `mixed $result` parameter between `$key` and `$response`. +This is **not** contravariant — every existing `onSuccess` closure must add the new positional parameters. | Before | After | |---|---| -| `onSuccess(fn(string $key, ResponseInterface $r))` | `onSuccess(fn(string $key, mixed $result, ResponseInterface $r))` | +| `onSuccess(fn(string $key, ResponseInterface $r))` | `onSuccess(fn(string $key, int $retries, mixed $result, ResponseInterface $r))` | ```php // before @@ -25,8 +25,12 @@ new BatchHttpClient() // after new BatchHttpClient() ->request([...]) - ->onSuccess(function (string $key, mixed $result, ResponseInterface $response): void { + ->onSuccess(function (string $key, int $retries, mixed $result, ResponseInterface $response): void { // $result is what's already in $results[$key] — post-parseResponse if configured + // $retries == 0 if first-attempt success; >= 1 if success came after retries + if ($retries > 0) { + log_warning("$key recovered after $retries retries"); + } log_success($key, $result); }) ->fetch(); @@ -72,28 +76,79 @@ The previous narrow catch left a hole: a 200 OK with malformed JSON killed the w ```php // before -->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { +->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { log_retry($key, $e); }) // after -->onRetry(function (string $key, int $attempt, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { +->onRetry(function (string $key, int $retries, ResponseInterface $failedResponse, ExceptionInterface|InvalidResponseException $e, ResponseInterface $retryResponse): void { log_retry($key, $e); }) ``` Closures that previously typed `$e` as just `\Throwable` or were untyped need no change — the new union is contravariant for them. -**Behavior change**: a 200 OK with malformed JSON now retries up to `maxRetries` and exhausts to `onExhausted` (or rethrows `JsonException` per `throwOnExhausted`), instead of firing `onAbort`. If you want the old "abort the batch on this exception" behavior for some specific case, throw a non-`ExceptionInterface` `Throwable` (e.g., `\RuntimeException`) from your `parseResponse` — `onAbort`'s outer catch is still `Throwable`, so it grabs anything outside the retry union. +**Behavior change**: a 200 OK with malformed JSON now retries up to `maxRetries` and exhausts to `onExhausted` (or rethrows `JsonException` per `throwOnExhausted`), instead of firing `onAbort`. If you want the old "abort the batch on this exception" behavior for some specific case, throw a non-`ExceptionInterface` `Throwable` (e.g., `\RuntimeException`) from your `parseResponse` — `onAbort`'s outer catch is still `Throwable`, so it grabs anything outside the retry union. (`onAbort`'s exception parameter type stays `Throwable`; its signature gains `$retries` as part of the standardization in section 5.) + +## 4. `onExhausted()` callback receives retry count + +`onExhausted` gains `int $retries` at position 2 — the actual retry count when exhaustion fired. Useful for distinguishing **normal exhaustion** (`$retries == maxRetries`) from **short-circuit exhaustion** (`$retries < maxRetries`, when a transport error fires with `retryOnTransportException: false` so the request bails before hitting the cap). + +| Before | After | +|---|---| +| `onExhausted(fn(string $key, ResponseInterface $r, ExceptionInterface\|InvalidResponseException $e))` | `onExhausted(fn(string $key, int $retries, ResponseInterface $r, ExceptionInterface\|InvalidResponseException $e))` | + +```php +// before +->onExhausted(function (string $key, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e): void { + log_exhausted($key, $e); +}) + +// after +->onExhausted(function (string $key, int $retries, ResponseInterface $response, ExceptionInterface|InvalidResponseException $e): void { + if ($retries < $maxRetries) { + log_warning("$key short-circuited after $retries retries (transport error with retryOnTransportException: false)"); + } + log_exhausted($key, $e); +}) +``` + +## 5. `onAbort()` callback receives retry count + +`onAbort` gains `int $retries` at position 2 — the retry count for the request being processed when the unexpected `Throwable` aborted the batch. Useful for distinguishing "aborted on the original attempt" from "aborted after N retries had already succeeded for the same request." + +Exception parameter type stays `Throwable` — `onAbort` is still the safety net for anything outside the retry union (`RuntimeException` from a user callback, `LogicException` from a `parseResponse` closure that throws something non-`ExceptionInterface`, etc.). + +| Before | After | +|---|---| +| `onAbort(fn(string $key, ResponseInterface $r, Throwable $e))` | `onAbort(fn(string $key, int $retries, ResponseInterface $r, Throwable $e))` | + +```php +// before +->onAbort(function (string $key, ResponseInterface $response, Throwable $e): void { + log_abort($key, $e); +}) + +// after +->onAbort(function (string $key, int $retries, ResponseInterface $response, Throwable $e): void { + log_abort($key, $retries, $e); +}) +``` + +**API consistency**: with this change, all four callbacks now share the `(string $key, int $retries, ...)` prefix. `$retries` semantics are uniform across them — "number of retries that have happened up to this point in the request's lifecycle." + +**Naming note (applies to `onRetry` only)**: examples and tests now consistently use `$retries` (instead of `$attempt`) on `onRetry` for parallelism. Closure parameter names are user-controlled, so no migration is required — existing closures using `$attempt` keep working unchanged. ## Finding callers before you upgrade -Grep for all three: +Grep for all five: ``` grep -rn '->onSuccess(' src tests +grep -rn '->onExhausted(' src tests +grep -rn '->onAbort(' src tests grep -rn 'throwOnError' src tests grep -rn 'TransportExceptionInterface|HttpExceptionInterface|InvalidResponseException' src tests ``` -For `onSuccess`, each match must add the `$result` parameter. For `throwOnError`, each match must rename to `throwOnExhausted`. For the retry-catch union typehint, each match must collapse to `ExceptionInterface|InvalidResponseException` (and import `Symfony\Contracts\HttpClient\Exception\ExceptionInterface` if needed). +For `onSuccess`, each match must add `int $retries` and `mixed $result` parameters. For `onExhausted` and `onAbort`, each match must add `int $retries` at position 2. For `throwOnError`, each match must rename to `throwOnExhausted`. For the retry-catch union typehint, each match must collapse to `ExceptionInterface|InvalidResponseException` (and import `Symfony\Contracts\HttpClient\Exception\ExceptionInterface` if needed).