fix(country): count Retry-After from the answer, not the asking #403

2026-10-17T20:41:46Z

rvnminers-A-and-N commented

2026-10-17 20:41:46 +00:00

Closes #402.

w.lookups.end(time.Now(), lookUpCountry(w.ctx)) read time.Now() before the lookup ran, since Go evaluates arguments left to right. So the service's Retry-After, and the backoff, were counted from the asking, not the answer. The next lookup could then come up to one round trip early. That's what made TestARateLimitsRetryAfterIsWaitedOut fail about one run in 18 (999.97 ms apart in #398's CI).

Fix. Take the answer, then the time, with a comment saying why the two are kept apart.

Test. The rate-limited answer now takes 200 ms, and the gap between the two requests must be at least 1.2 s. Without the fix it fails every time. With it, 180 runs under -race at 1, 4 and 8 CPUs all pass, as do the other country-lookup tests.

No new branches, so coverage is unchanged. No docs change: --locate-country already says the wait follows the service's Retry-After.

Closes #402. `w.lookups.end(time.Now(), lookUpCountry(w.ctx))` read `time.Now()` before the lookup ran, since Go evaluates arguments left to right. So the service's `Retry-After`, and the backoff, were counted from the asking, not the answer. The next lookup could then come up to one round trip early. That's what made `TestARateLimitsRetryAfterIsWaitedOut` fail about one run in 18 (999.97 ms apart in #398's CI). **Fix.** Take the answer, then the time, with a comment saying why the two are kept apart. **Test.** The rate-limited answer now takes 200 ms, and the gap between the two requests must be at least 1.2 s. Without the fix it fails every time. With it, 180 runs under `-race` at 1, 4 and 8 CPUs all pass, as do the other country-lookup tests. No new branches, so coverage is unchanged. No docs change: `--locate-country` already says the wait follows the service's Retry-After.

rvnminers-A-and-N commented

2026-10-17 20:42:25 +00:00

Closing in favour of #401, which @excho0 opened a few minutes earlier with the same fix. #402 records the cause; #401 closes it.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Sisyphus/Sisyphus#403