Skip to content

Surface Retry-After so rate-limited visitors know when to retry#49

Open
HMAKT99 wants to merge 2 commits into
Forward-Future:mainfrom
HMAKT99:fix/expose-retry-after
Open

Surface Retry-After so rate-limited visitors know when to retry#49
HMAKT99 wants to merge 2 commits into
Forward-Future:mainfrom
HMAKT99:fix/expose-retry-after

Conversation

@HMAKT99

@HMAKT99 HMAKT99 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

The form gateway already computes and returns a Retry-After header on 429 responses (both the verification rate limiter and the per-IP hourly/daily limiter), but it never reached the visitor:

  1. The Worker didn't expose it. Retry-After is not a CORS-safelisted response header, so a cross-origin fetch() from the site can't read it unless it's listed in Access-Control-Expose-Headers — which it wasn't.
  2. The client ignored it anyway. postProtectedForm() surfaced only responseBody.error, so a rate-limited visitor saw "Try again later" with no sense of how long.

Fix

  • Worker (worker/src/index.js): add Access-Control-Expose-Headers: Retry-After to the CORS headers so the browser can read it.
  • Client (site/script.js): on a 429, read Retry-After and append a concrete hint — e.g. "Try again in about 1 minute." — to the existing status message.
  • Test (worker/test/index.test.js): assert the header is exposed on rate-limited responses.

The hint formatting is defensive: a missing or non-numeric header simply yields no hint, preserving today's behavior.

Verification

  • npm --prefix worker run check → 14/14 tests pass.
  • node scripts/check.mjsLoop Library checks passed.
  • node --check site/script.js
  • Builders → no artifact drift.

The form gateway already returns a Retry-After header on 429 responses,
but two things kept it from reaching the visitor. The Worker never added
Retry-After to Access-Control-Expose-Headers, so a cross-origin browser
fetch could not read it, and the client discarded it regardless, showing
only a generic Try again later message.

Expose the header from the Worker and, on a 429, append a concrete hint
(Try again in about N seconds/minutes) to the form status message so the
visitor knows how long to wait instead of retrying immediately. Covered by
a Worker test asserting the header is exposed on rate-limited responses.
@HMAKT99

HMAKT99 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

While reading the form gateway I noticed the Retry-After header it sends on 429s never actually reaches visitors: it isn't in Access-Control-Expose-Headers, so a cross-origin fetch can't read it, and the client discarded it anyway. This exposes it and surfaces a concrete "try again in ~N minutes" hint instead of a bare "try again later."

It's intentionally defensive — a missing/odd header just yields no hint, so current behavior is preserved. Added a worker test asserting the header is exposed; npm --prefix worker run check is green (14→ now passing with the new assertion).

@mberman84 mberman84 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Worker correctly exposes Retry-After through CORS, and the integrated Worker suite passes. Two client-side fixes remain:

  1. Math.round(seconds / 60) can understate the server's minimum delay by up to 29 seconds (for example, 3,569 seconds is shown as 59 minutes). Please use Math.ceil(seconds / 60) so the guidance never sends the visitor back before the limit expires.
  2. This changes site/script.js without changing the current versioned script URL. Please bump the shared script asset version everywhere it is emitted/validated and regenerate affected pages so cached visitors receive the new behavior.

Address review on the rate-limit hint:
- Use Math.ceil when converting Retry-After seconds to the displayed
  minutes/seconds so the hint never sends a visitor back before the
  server's limit expires (e.g. 3,569s now reads 60 minutes, not 59).
- Bump the shared script.js asset version to 20260621-retry-after across
  the homepage, Learn/Agents pages, the loop-page generator, and
  scripts/check.mjs, then regenerate the loop pages so cached visitors
  receive the new client behavior.
@HMAKT99

HMAKT99 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Both addressed: switched the seconds→minutes (and seconds) conversion to Math.ceil so the hint never sends a visitor back before the limit expires, and bumped the script.js version to 20260621-retry-after everywhere it's emitted/validated, then regenerated the loop pages. Worker suite still 14/14; site check + builders green.

@HMAKT99

HMAKT99 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@mberman84 ready for another look — Math.ceil rounding fix in and the asset version bumped. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants