fix(providers): treat empty Retry-After header as no hint#91
Merged
Conversation
Number('') and Number(' ') coerce to 0, so an empty or whitespace-only
Retry-After header was being read as a 0 ms hint. The downstream
buildRetryInfo guards against zero by Math.max'ing against the backoff,
so behavior was not catastrophic, but the classifier was emitting a
misleading retryAfterMs=0 that surfaced via onRetry callbacks.
Reject empty strings up front, gate the seconds path on a numeric-only
regex, and let HTTP-date Retry-After values fall through to Date.parse.
Adds three tests covering empty, whitespace, and HTTP-date cases.
Contributor
There was a problem hiding this comment.
Findings
- No high-confidence issues found in the modified lines.
Summary
- Review mode: initial
- No issues identified in this diff.
- Constraint check on modified lines: no direct provider SDK imports, no new dependencies/licenses, no hardcoded UI tokens, and no silent fallback introduced.
- Not found in repo/docs:
docs/VISION.md,docs/PRINCIPLES.md(repository note indicates internal/gitignored docs), so full policy cross-check against those files could not be completed from this checkout. - Residual risk/testing gap: HTTP-date parsing test is time-window based and may be flaky under heavy CI clock skew; no deterministic time-mocking in added tests.
Testing
- Not run (automation)
open-codesign Bot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
extractRetryAfterMswas passing the raw header toNumber(...), so an empty/whitespace-onlyRetry-Aftercoerced to0and emitted a misleadingretryAfterMs=0hint toonRetrycallbacks (downstreambuildRetryInfoclamps to backoff so behavior wasn't catastrophic, but the signal was wrong).Date.parsefor HTTP-dateRetry-Aftervalues.Changes
packages/providers/src/retry.ts— guard empty/whitespace input, numeric regex beforeNumber(...).packages/providers/src/retry.test.ts— three new tests: empty header, whitespace-only header, HTTP-date header.PRINCIPLES check
Test plan
pnpm --filter @open-codesign/providers test— 39 passed (was 36).pnpm --filter @open-codesign/providers typecheck.pnpm exec biome check packages/providers/src/retry.ts packages/providers/src/retry.test.ts.