Skip to content

Commit f3e7700

Browse files
evan-burrellSammyjo20
evan-burrell
and
Sammyjo20
authored
Fix exceeded() to fall back to configured interval when Retry-After is missing (#28)
* Fix exceeded() to always set expiry timestamp by falling back to configured interval When `exceeded()` is called without a `releaseInSeconds` value (e.g. when a 429 response has no Retry-After header), the expiry timestamp was not set explicitly. This caused it to be lazily calculated from "now" on each call to `getExpiryTimestamp()`, which can drift and lead to inconsistent state. Now `exceeded()` falls back to the limit's configured `releaseInSeconds` when no explicit value is provided, ensuring the expiry timestamp is always set deterministically at the moment the limit is exceeded. * Fix flaky timestamp assertions with custom Pest expectation Add a `toLookLike` custom expectation that tolerates ±1 second drift on timestamps, preventing flaky failures when a second boundary is crossed between capturing the expected time and executing the request. --------- Co-authored-by: Sam Carré <29132017+Sammyjo20@users.noreply.github.com>
1 parent bdd7bf1 commit f3e7700

4 files changed

Lines changed: 66 additions & 19 deletions

File tree

‎src/Limit.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,10 @@ public function exceeded(?int $releaseInSeconds = null): void
130130

131131
$this->hits = $this->allow;
132132

133-
if (isset($releaseInSeconds)) {
134-
$interval = DateInterval::createFromDateString($releaseInSeconds . ' seconds');
133+
$seconds = $releaseInSeconds ?? $this->releaseInSeconds;
134+
135+
if ($seconds > 0) {
136+
$interval = DateInterval::createFromDateString($seconds . ' seconds');
135137

136138
if ($interval === false) {
137139
return;

‎tests/Feature/HasRateLimitsTest.php‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@
5151
expect($storeData)->toHaveKey('TestConnector:3_every_60');
5252
expect($storeData)->toHaveKey('TestConnector:too_many_attempts_limit');
5353

54-
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toEqual([
54+
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toLookLike([
5555
'hits' => 1,
5656
'timestamp' => $currentTimestampPlusSixty,
5757
]);
5858

59-
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toEqual([
59+
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toLookLike([
6060
'hits' => 0,
6161
'allow' => 1,
6262
'timestamp' => $currentTimestampPlusSixty,
@@ -69,12 +69,12 @@
6969

7070
$storeData = $store->getStore();
7171

72-
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toEqual([
72+
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toLookLike([
7373
'hits' => 2,
7474
'timestamp' => $currentTimestampPlusSixty,
7575
]);
7676

77-
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toEqual([
77+
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toLookLike([
7878
'hits' => 0,
7979
'allow' => 1,
8080
'timestamp' => $currentTimestampPlusSixty,
@@ -87,12 +87,12 @@
8787

8888
$storeData = $store->getStore();
8989

90-
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toEqual([
90+
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toLookLike([
9191
'hits' => 3,
9292
'timestamp' => $currentTimestampPlusSixty,
9393
]);
9494

95-
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toEqual([
95+
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toLookLike([
9696
'hits' => 0,
9797
'allow' => 1,
9898
'timestamp' => $currentTimestampPlusSixty,
@@ -145,7 +145,7 @@
145145
expect($storeData)->toHaveKey('TestConnector:3_every_5');
146146
expect($storeData)->toHaveKey('TestConnector:too_many_attempts_limit');
147147

148-
expect(parseRawLimit($storeData['TestConnector:3_every_5']))->toEqual([
148+
expect(parseRawLimit($storeData['TestConnector:3_every_5']))->toLookLike([
149149
'hits' => 1,
150150
'timestamp' => $currentTimestampPlusFive,
151151
]);
@@ -157,7 +157,7 @@
157157

158158
$storeData = $store->getStore();
159159

160-
expect(parseRawLimit($storeData['TestConnector:3_every_5']))->toEqual([
160+
expect(parseRawLimit($storeData['TestConnector:3_every_5']))->toLookLike([
161161
'hits' => 2,
162162
'timestamp' => $currentTimestampPlusFive,
163163
]);
@@ -169,7 +169,7 @@
169169

170170
$storeData = $store->getStore();
171171

172-
expect(parseRawLimit($storeData['TestConnector:3_every_5']))->toEqual([
172+
expect(parseRawLimit($storeData['TestConnector:3_every_5']))->toLookLike([
173173
'hits' => 3,
174174
'timestamp' => $currentTimestampPlusFive,
175175
]);
@@ -344,12 +344,12 @@
344344
expect($storeData)->toHaveKey('LimitedRequest:60_every_60');
345345
expect($storeData)->toHaveKey('LimitedRequest:too_many_attempts_limit');
346346

347-
expect(parseRawLimit($storeData['LimitedRequest:60_every_60']))->toEqual([
347+
expect(parseRawLimit($storeData['LimitedRequest:60_every_60']))->toLookLike([
348348
'hits' => 1,
349349
'timestamp' => $currentTimestampPlusSixty,
350350
]);
351351

352-
expect(parseRawLimit($storeData['LimitedRequest:too_many_attempts_limit']))->toEqual([
352+
expect(parseRawLimit($storeData['LimitedRequest:too_many_attempts_limit']))->toLookLike([
353353
'hits' => 0,
354354
'allow' => 1,
355355
'timestamp' => $currentTimestampPlusSixty,
@@ -390,23 +390,23 @@
390390
expect($storeData)->toHaveKey('TestConnector:3_every_60');
391391
expect($storeData)->toHaveKey('TestConnector:too_many_attempts_limit');
392392

393-
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toEqual([
393+
expect(parseRawLimit($storeData['TestConnector:3_every_60']))->toLookLike([
394394
'hits' => 1,
395395
'timestamp' => $currentTimestampPlusSixty,
396396
]);
397397

398-
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toEqual([
398+
expect(parseRawLimit($storeData['TestConnector:too_many_attempts_limit']))->toLookLike([
399399
'hits' => 0,
400400
'allow' => 1,
401401
'timestamp' => $currentTimestampPlusSixty,
402402
]);
403403

404-
expect(parseRawLimit($storeData['LimitedRequest:60_every_60']))->toEqual([
404+
expect(parseRawLimit($storeData['LimitedRequest:60_every_60']))->toLookLike([
405405
'hits' => 1,
406406
'timestamp' => $currentTimestampPlusSixty,
407407
]);
408408

409-
expect(parseRawLimit($storeData['LimitedRequest:too_many_attempts_limit']))->toEqual([
409+
expect(parseRawLimit($storeData['LimitedRequest:too_many_attempts_limit']))->toLookLike([
410410
'hits' => 0,
411411
'allow' => 1,
412412
'timestamp' => $currentTimestampPlusSixty,
@@ -443,12 +443,12 @@
443443
expect($storeData)->toHaveKey('LimitedSoloRequest:60_every_60');
444444
expect($storeData)->toHaveKey('LimitedSoloRequest:too_many_attempts_limit');
445445

446-
expect(parseRawLimit($storeData['LimitedSoloRequest:60_every_60']))->toEqual([
446+
expect(parseRawLimit($storeData['LimitedSoloRequest:60_every_60']))->toLookLike([
447447
'hits' => 1,
448448
'timestamp' => $currentTimestampPlusSixty,
449449
]);
450450

451-
expect(parseRawLimit($storeData['LimitedSoloRequest:too_many_attempts_limit']))->toEqual([
451+
expect(parseRawLimit($storeData['LimitedSoloRequest:too_many_attempts_limit']))->toLookLike([
452452
'hits' => 0,
453453
'allow' => 1,
454454
'timestamp' => $currentTimestampPlusSixty,

‎tests/Pest.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,22 @@ function parseRawLimit(?string $data): ?array
4444
return ! empty($data) ? json_decode($data, true) : null;
4545
}
4646

47+
expect()->extend('toLookLike', function (array $expected) {
48+
$actual = $this->value;
49+
50+
$expectedTimestamp = $expected['timestamp'];
51+
unset($expected['timestamp']);
52+
53+
$actualTimestamp = $actual['timestamp'];
54+
unset($actual['timestamp']);
55+
56+
expect($actual)->toEqual($expected);
57+
expect($actualTimestamp)->toBeGreaterThanOrEqual($expectedTimestamp);
58+
expect($actualTimestamp)->toBeLessThanOrEqual($expectedTimestamp + 1);
59+
60+
return $this;
61+
});
62+
4763
/**
4864
* Reset the testing directory
4965
*/

‎tests/Unit/LimitTest.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,32 @@
100100

101101
expect($limit->getReleaseInSeconds())->toEqual($seconds);
102102
});
103+
104+
test('exceeded without releaseInSeconds falls back to the configured interval', function () {
105+
$limit = Limit::allow(10)->everySeconds(120);
106+
107+
$limit->exceeded();
108+
109+
expect($limit->wasManuallyExceeded())->toBeTrue()
110+
->and($limit->getHits())->toBe(10)
111+
->and($limit->getRemainingSeconds())->toBe(120);
112+
});
113+
114+
test('exceeded with explicit releaseInSeconds uses the provided value', function () {
115+
$limit = Limit::allow(10)->everySeconds(120);
116+
117+
$limit->exceeded(releaseInSeconds: 300);
118+
119+
expect($limit->wasManuallyExceeded())->toBeTrue()
120+
->and($limit->getRemainingSeconds())->toBe(300);
121+
});
122+
123+
test('custom limiter exceeded without releaseInSeconds falls back to default 60 seconds', function () {
124+
$limit = Limit::custom(function () {
125+
});
126+
127+
$limit->exceeded();
128+
129+
expect($limit->wasManuallyExceeded())->toBeTrue()
130+
->and($limit->getRemainingSeconds())->toBe(60);
131+
});

0 commit comments

Comments
 (0)