Country discovery now starts the server-provided Retry-After window after receiving the HTTP response, rather than before the lookup request begins. A deliberately delayed rate-limit response tests that the full interval is honored from the response.
The full integration suite exposed this existing timing bug while checking #398. It explains the near-one-second assertion failure and prevents retrying earlier than the provider requested.
Verification
go vet ./apps/sisyphusd
go test -race ./apps/sisyphusd -run '^(TestARateLimitsRetryAfterIsWaitedOut|TestRetryAfterIsReadInSecondsOrAsADate|TestTheCountryIsLookedUpAgainAfterATransientFailure)$' -count=2
## What changed
Country discovery now starts the server-provided `Retry-After` window after receiving the HTTP response, rather than before the lookup request begins. A deliberately delayed rate-limit response tests that the full interval is honored from the response.
The full integration suite exposed this existing timing bug while checking #398. It explains the near-one-second assertion failure and prevents retrying earlier than the provider requested.
## Verification
- `go vet ./apps/sisyphusd`
- `go test -race ./apps/sisyphusd -run '^(TestARateLimitsRetryAfterIsWaitedOut|TestRetryAfterIsReadInSecondsOrAsADate|TestTheCountryIsLookedUpAgainAfterATransientFailure)$' -count=2`
- `git diff --check`
Part of #393.
Same cause I tracked down independently (#402, my now-closed #403): time.Now() is evaluated before lookUpCountry runs, so Retry-After counted from the asking. Without the fix, the test failed 10 times in 180 local runs, with gaps as short as 997.6 ms. Measuring from responseAt makes the assertion exact. Approving. Could you add Closes #402 to the description so the issue closes with it?
Same cause I tracked down independently (#402, my now-closed #403): `time.Now()` is evaluated before `lookUpCountry` runs, so Retry-After counted from the asking. Without the fix, the test failed 10 times in 180 local runs, with gaps as short as 997.6 ms. Measuring from `responseAt` makes the assertion exact. Approving. Could you add `Closes #402` to the description so the issue closes with it?
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
rvnminers-A-and-N
What changed
Country discovery now starts the server-provided
Retry-Afterwindow after receiving the HTTP response, rather than before the lookup request begins. A deliberately delayed rate-limit response tests that the full interval is honored from the response.The full integration suite exposed this existing timing bug while checking #398. It explains the near-one-second assertion failure and prevents retrying earlier than the provider requested.
Verification
go vet ./apps/sisyphusdgo test -race ./apps/sisyphusd -run '^(TestARateLimitsRetryAfterIsWaitedOut|TestRetryAfterIsReadInSecondsOrAsADate|TestTheCountryIsLookedUpAgainAfterATransientFailure)$' -count=2git diff --checkPart of #393.
Same cause I tracked down independently (#402, my now-closed #403):
time.Now()is evaluated beforelookUpCountryruns, so Retry-After counted from the asking. Without the fix, the test failed 10 times in 180 local runs, with gaps as short as 997.6 ms. Measuring fromresponseAtmakes the assertion exact. Approving. Could you addCloses #402to the description so the issue closes with it?