Repository navigation
test(adapters/linear): fix TestRateLimitDeadlineReturnsTypedRateLimited flake under -race load - #110
Conversation
…ed flake under -race load The test used one 50ms real-clock context as both the transport context and the reference deadline for the no-sleep invariant. PostThought makes two localhost roundtrips inside that window (the OAuth client-credentials token fetch, then the GraphQL POST that gets the 429). Under -race CPU load either roundtrip can exceed 50ms, so the client returned a plain "context deadline exceeded" before the rate-limit path ran. The deadline branch in doJSONWithRetry fires whenever now+delay (one hour here) overshoots the caller deadline, so any deadline far below an hour exercises it identically. Widen the deadline to 10s and let a 5s completion bound separate "returned promptly" from "slept until the deadline or backoff". Same fix as the Slack flake in #39. No production change: returning the transport context error when the deadline expires before the 429 arrives is correct. Red check: disabling the deadline branch in retry.go makes the test fail with "slept past its caller deadline". Loaded repro (48 parallel -race -count=50 -cpu=1 instances plus 32 CPU spinners, 3 rounds): 316/7200 failures before, 0/7200 after. Fixes #109
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Fixes #109
Cause
TestRateLimitDeadlineReturnsTypedRateLimitedgavePostThoughta 50 ms real-clock context. That one context was both the transport context and the reference deadline for the no-sleep check.PostThoughtmakes two localhost roundtrips inside that window: the OAuth client-credentials token fetch, then the GraphQL POST that receives the 429. Under-racewith CPU load, either roundtrip can take longer than 50 ms. The client then returns a plaincontext deadline exceededbefore the rate-limit path runs. The failure logs show both forms:Post ".../graphql": context deadline exceededandfetch client credentials token: Post ".../oauth/token": context deadline exceeded.The production code is correct. If the deadline expires before the 429 arrives, returning the transport's context error is the right result.
Fix (test only)
This PR uses the same fix as the Slack flake in #39 (PR #41):
doJSONWithRetryfires whenevernow + delay(one hour here) passes the caller deadline. Any deadline far below one hour exercises that branch the same way. 10 s is about 3 orders of magnitude above a loaded roundtrip and about 3 below the one-hour sleep the test guards against.The assertion is unchanged: the error must be
*linear.RateLimited.Red check: I disabled the deadline branch in
retry.go(false && ok && ...). The test then fails withPostThought slept past its caller deadline: a long backoff was not deadline-bounded.Failure rates
The parallel repro is 48 parallel instances of the
-racetest binary, each with-count=50 -cpu=1, plus 32 busy-loop CPU spinners, on a 32-CPU host:go test -race -count=200 -cpu=1 -run TestRateLimitDeadlineReturnsTypedRateLimited ./adapters/linear/under the 32 spinnersValidation
mise run vetandmise run testpass with Go 1.27.1.gofmt -lis clean.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high