Skip to content

Commit f7a59ed

Browse files
authored
Fix | Allow the "Too Many Attempts" limiter to have sleep() (#27)
* Fix | Allow the "Too Many Attempts" limiter to have sleep() * Fixed flakey test
1 parent 67e82ca commit f7a59ed

File tree

4 files changed

+79
-10
lines changed

4 files changed

+79
-10
lines changed

src/Helpers/LimitHelper.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace Saloon\RateLimitPlugin\Helpers;
66

7-
use Closure;
87
use Saloon\RateLimitPlugin\Limit;
98
use Saloon\RateLimitPlugin\Exceptions\LimitException;
109

@@ -17,7 +16,7 @@ class LimitHelper
1716
* @return array<Limit>
1817
* @throws LimitException
1918
*/
20-
public static function configureLimits(array $limits, ?string $prefix, ?Closure $tooManyAttemptsHandler = null): array
19+
public static function configureLimits(array $limits, ?string $prefix, ?Limit $tooManyAttemptsLimit = null): array
2120
{
2221
// Firstly, we will clean up the limits array to only ensure the `Limit` classes
2322
// are being processed.
@@ -33,8 +32,8 @@ public static function configureLimits(array $limits, ?string $prefix, ?Closure
3332
// Next we will append our "too many attempts" limit which will be used when
3433
// the response actually hits a 429 status.
3534

36-
if (isset($tooManyAttemptsHandler)) {
37-
$limits[] = Limit::custom($tooManyAttemptsHandler)->name('too_many_attempts_limit');
35+
if (isset($tooManyAttemptsLimit)) {
36+
$limits[] = $tooManyAttemptsLimit->name('too_many_attempts_limit');
3837
}
3938

4039
// Next we will set the prefix on each of the limits.

src/Traits/HasRateLimits.php

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public function bootHasRateLimits(PendingRequest $pendingRequest): void
5454
}
5555
});
5656

57-
$pendingRequest->middleware()->onResponse(function (Response $response): void {
57+
$pendingRequest->middleware()->onResponse(function (Response $response): Response {
5858
$limitThatWasExceeded = null;
5959
$store = $this->rateLimitStore();
6060

@@ -92,8 +92,18 @@ public function bootHasRateLimits(PendingRequest $pendingRequest): void
9292
// place. We should make sure to throw the exception here.
9393

9494
if (isset($limitThatWasExceeded)) {
95-
$this->throwLimitException($limitThatWasExceeded);
95+
if (! $limit->getShouldSleep()) {
96+
$this->throwLimitException($limitThatWasExceeded);
97+
}
98+
99+
// When the limit has been instructed to sleep() we will make the request
100+
// again, which will trigger the request middleware to sleep, and then
101+
// hopefully get a successful response afterward.
102+
103+
return $this->send($response->getRequest());
96104
}
105+
106+
return $response;
97107
}, order: PipeOrder::FIRST);
98108
}
99109

@@ -105,6 +115,16 @@ protected function getLimiterPrefix(): ?string
105115
return (new ReflectionClass($this))->getShortName();
106116
}
107117

118+
/**
119+
* Define the "Too Many Attempts" (429) limiter
120+
*
121+
* This limiter will automatically attempt to detect 429 requests.
122+
*/
123+
protected function getTooManyAttemptsLimiter(): ?Limit
124+
{
125+
return Limit::custom($this->handleTooManyAttempts(...));
126+
}
127+
108128
/**
109129
* Handle too many attempts (429) statuses
110130
*/
@@ -209,9 +229,9 @@ public function rateLimitStore(): RateLimitStore
209229
*/
210230
public function getLimits(): array
211231
{
212-
$tooManyAttemptsHandler = $this->detectTooManyAttempts === true ? $this->handleTooManyAttempts(...) : null;
232+
$tooManyAttemptsLimit = $this->detectTooManyAttempts === true ? $this->getTooManyAttemptsLimiter() : null;
213233

214-
return LimitHelper::configureLimits($this->resolveLimits(), $this->getLimiterPrefix(), $tooManyAttemptsHandler);
234+
return LimitHelper::configureLimits($this->resolveLimits(), $this->getLimiterPrefix(), $tooManyAttemptsLimit);
215235
}
216236

217237
/**

tests/Feature/HasRateLimitsTest.php

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Saloon\Exceptions\Request\Statuses\InternalServerErrorException;
1717
use Saloon\RateLimitPlugin\Tests\Fixtures\Requests\LimitedSoloRequest;
1818
use Saloon\RateLimitPlugin\Tests\Fixtures\Connectors\CustomPrefixConnector;
19+
use Saloon\RateLimitPlugin\Tests\Fixtures\Connectors\SleepTooManyRequestsConnector;
1920
use Saloon\RateLimitPlugin\Tests\Fixtures\Connectors\CustomTooManyRequestsConnector;
2021
use Saloon\RateLimitPlugin\Tests\Fixtures\Connectors\DisabledTooManyRequestsConnector;
2122

@@ -173,11 +174,11 @@
173174
'timestamp' => $currentTimestampPlusFive,
174175
]);
175176

176-
// Now when we make this request, it should pause the application for 10 seconds
177+
// Now when we make this request, it should pause the application for 5 seconds
177178

178179
$connector->send(new UserRequest);
179180

180-
expect(time())->toEqual($currentTimestampPlusFive);
181+
expect(time())->toBeGreaterThanOrEqual($currentTimestampPlusFive);
181182
});
182183

183184
test('you can create a limiter that listens for 429 and will automatically back off for the Retry-After duration', function () {
@@ -293,6 +294,27 @@
293294
expect($store->getStore())->toBeEmpty();
294295
});
295296

297+
test('you can customise the 429 error detection to sleep instead', function () {
298+
$store = new MemoryStore;
299+
$connector = new SleepTooManyRequestsConnector($store, []);
300+
301+
$connector->withMockClient(new MockClient([
302+
new MockResponse(['name' => 'Sam'], 200),
303+
MockResponse::make(['status' => 'Too Many Requests'], 429),
304+
MockResponse::make(['name' => 'Jon'], 200),
305+
]));
306+
307+
$startTime = time();
308+
309+
$connector->send(new UserRequest);
310+
311+
$response = $connector->send(new UserRequest);
312+
313+
expect($response->json())->toBe(['name' => 'Jon']);
314+
315+
expect(time())->toBeGreaterThanOrEqual($startTime + 5);
316+
});
317+
296318
test('the rate limiter can be used on a request', function () {
297319
$store = new MemoryStore;
298320

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Saloon\RateLimitPlugin\Tests\Fixtures\Connectors;
6+
7+
use Saloon\Http\Response;
8+
use Saloon\RateLimitPlugin\Limit;
9+
10+
class SleepTooManyRequestsConnector extends TestConnector
11+
{
12+
protected function getTooManyAttemptsLimiter(): ?Limit
13+
{
14+
return Limit::custom($this->handleTooManyAttempts(...))->sleep();
15+
}
16+
17+
/**
18+
* Handle too many attempts (429) statuses
19+
*/
20+
protected function handleTooManyAttempts(Response $response, Limit $limit): void
21+
{
22+
if ($response->status() !== 429) {
23+
return;
24+
}
25+
26+
$limit->exceeded(releaseInSeconds: 5);
27+
}
28+
}

0 commit comments

Comments
 (0)