[d-klotz]

The Requester now waits as long as the provider says when it throttles us, and an API module can declare its provider's limits. A wait that is too long to sleep in process throws a typed RateLimitError with retryAt. This is phase 1 of ADR-049 (#655).

What problem this fixes

• The Requester ignores what the provider says. On a 429 it walks one fixed ladder (1, 3, 10, 30, 60 and 180 s) whatever Retry-After says. A provider that says "call again in 2 s" still gets the 180 s step. A provider that says "call again in 10 minutes" gets six attempts and then a thrown FetchError with no time on it.
• A limit that is not a 429 is lost. A provider that signals its daily limit with a 403 loses the message: the queue worker halts every 4xx except 408 and 429, and discards the message.
• Nothing above the Requester can wait for the right time. The thrown error carries no time, so no caller can act on it.

What this PR does

• Honours Retry-After on a 429, for every module. Delta seconds, an HTTP-date and an ISO timestamp.
• Lets an API module declare static rateLimit. Every key is optional:
  • classify({ status, headers, body }) recognises a limit that is not a plain 429 (a 403 limit code, a policyName in the body) and may name only a reason;
  • parsers picks the header parsers (retryAfter, resetHeaders, ietf; default all three);
  • minRetryAfterMs is a floor for the wait ("wait 60 seconds" in the docs, nothing in the response);
  • maxInProcessWaitMs (default 5 minutes) and scope.
• Sleeps in process only while the wait fits. One request may sleep 5 minutes in total, and never longer than the time left in the Lambda invocation less a reserve for the next request: the request timeout, or half the time left when that is smaller. A 2 s or 10 s wait therefore sleeps even in a 29 s HTTP Lambda. A wait that does not fit throws RateLimitError.
• RateLimitError extends FetchError. It keeps statusCode and adds isRateLimited, retryAt, waitMs, reason, policy, source, module and scopeKey.
• The queue worker no longer halts an error with isRateLimited, so a 403 limit goes back to SQS instead of being discarded.
• Exports classifyRateLimit for a module whose client is not the Requester (for example one built on a vendor SDK).

class Api extends OAuth2Requester {
    static rateLimit = {
        minRetryAfterMs: 1_000,
        classify({ status, body }) {
            if (status === 403 && body?.code === 'DAILY_LIMIT') {
                return { reason: 'daily', waitMs: 60 * 60_000 };
            }
            return null;
        },
    };
}

How a throttled response is read

1. The module's classify(). A status other than 429 counts only when classify returns a hint for it. A hint names a known reason, a waitMs or a retryAt; any other object is not a hint, and a returned Promise is logged as a classify error and ignored.
2. The header parsers, in order.
3. The static policy: the larger of minRetryAfterMs and the perMs of the window named by the reason.
4. The fixed ladder.

With a hint, the wait is the largest of the hint, minRetryAfterMs and 1 s, plus at most 10 % jitter. Jitter only adds and never passes the budget.

Compatibility

• A 429 with no hint behaves exactly as today: the same calls, the same delays, then a plain FetchError. The ladder is not budgeted (the provider's default HTTP Lambda timeout is 29 s, so a budget would have made a bare 429 throw on the first attempt). A test replays it inside a 29 s invocation.
• Retry-After is read on a 429 and on a response that classify recognises. A 503 that classify does not recognise keeps the 5xx ladder, Retry-After or not.
• backOff: [] still means no retries. The Requester then never sleeps for a hint either: a hint throws RateLimitError at once.
• Logs. A hinted wait writes one TRACE record (<logger>.rate_limited). The Requester writes no record for the error it throws; the boundary logs it once, and the logger now adds retryAt and reason to a rate-limited error.
• classify gets the body parsed as JSON whatever its media type (text/plain and application/x-amz-json-1.1 too); a body that is not JSON arrives as undefined.
• A hinted wait can be longer or shorter than the ladder step it replaces. That is the point. The cap on hinted waits, 5 minutes, is close to the total of today's ladder (about 284 s).
• classify is not called for a 2xx. A provider that answers 200 with a "throttled" body wraps its call with classifyRateLimit and throws RateLimitError itself.
• Hinted 429s can retry more than six times. The 1 s minimum wait and the cumulative cap bound the loop: about 300 attempts at most.
• The invocation deadline lives in its own AsyncLocalStorage, not in the logs store: the telemetry bus forwards every non-log key of that store. Outside a Lambda (tests, frigg start, scripts) there is no deadline and the cap alone applies.
• FetchError.create now builds the class it is called on and uses a responseBody it is given instead of reading the stream a second time. options.body stays a fallback after the stream, as before.

Not in this PR

• packages/schemas still describes an old rateLimit: { requests, period } shape that the runtime does not read. The reference doc says so; the schema needs its own change.
• Deferring a rate-limited message in the queue worker. RateLimitError still goes back to SQS as any retryable error does.
• The user-facing RATE_LIMITED message, pacing, and the policies of the HubSpot, Salesforce and Pipedrive modules.

Test plan

• core: npm test in packages/core — 2,720 pass; the 63 failures (DocumentDB, Prisma decryption, Mongo memory server) are the same 63 that fail on origin/next in this environment
• new suites: parsers, policy (read, resolve, wait, budget), RateLimitError, invocation deadline, public API
• Requester: replaces the two skipped tests that called a live mock server; a bare 429 keeps the exact ladder [1, 3, 10, 30, 60, 180] s and 7 calls, also under a 29 s deadline; a 2 s Retry-After sleeps inside a 29 s invocation; Retry-After wins over a ladder step; the cumulative cap; the deadline budget; classify for a 403, an async classify, JSON under any media type; backOff: []; the body is read once
• halt rule: a 403 with isRateLimited goes back to SQS
• type declarations checked with tsc --strict against a consumer, including two @ts-expect-error negatives
• eslint: 0 errors
• local end-to-end run (frigg-local-test harness: real Postgres, LocalStack SQS and Scheduler, a real QueueWorker from createQueueWorker, and an integration whose API module calls a local fake provider): a 429 with Retry-After: 1 sleeps in process and the call then succeeds; a 403 limit body sent as text/plain is classified and throws RateLimitError

Release notes

• An app changes behaviour only after it upgrades @friggframework/core and redeploys. A module that declares nothing keeps its behaviour, except that a 429 with Retry-After now waits as long as the header says.
• Upgrade @friggframework/core before an API module uses static rateLimit or RateLimitError.

🤖 Generated with Claude Code

📦 Published PR as canary version: 2.0.0--canary.656.602fc50.0

✨ Test out this PR locally via:

npm install @friggframework/admin-scripts@2.0.0--canary.656.602fc50.0
npm install @friggframework/core@2.0.0--canary.656.602fc50.0
npm install @friggframework/devtools@2.0.0--canary.656.602fc50.0
npm install @friggframework/eslint-config@2.0.0--canary.656.602fc50.0
npm install @friggframework/prettier-config@2.0.0--canary.656.602fc50.0
npm install @friggframework/schemas@2.0.0--canary.656.602fc50.0
npm install @friggframework/serverless-plugin@2.0.0--canary.656.602fc50.0
npm install @friggframework/test@2.0.0--canary.656.602fc50.0
npm install @friggframework/ui@2.0.0--canary.656.602fc50.0
# or 
yarn add @friggframework/admin-scripts@2.0.0--canary.656.602fc50.0
yarn add @friggframework/core@2.0.0--canary.656.602fc50.0
yarn add @friggframework/devtools@2.0.0--canary.656.602fc50.0
yarn add @friggframework/eslint-config@2.0.0--canary.656.602fc50.0
yarn add @friggframework/prettier-config@2.0.0--canary.656.602fc50.0
yarn add @friggframework/schemas@2.0.0--canary.656.602fc50.0
yarn add @friggframework/serverless-plugin@2.0.0--canary.656.602fc50.0
yarn add @friggframework/test@2.0.0--canary.656.602fc50.0
yarn add @friggframework/ui@2.0.0--canary.656.602fc50.0

[netlify]

✅ Deploy Preview for friggframework-org canceled.

Name	Link
🔨 Latest commit	602fc50
🔍 Latest deploy log	https://app.netlify.com/projects/friggframework-org/deploys/6af23ddda2706f6942b45a8c

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-28T23:21:28.346284Z	10f1676	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10f1676a00

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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".

[claude]

Findings marked 🟡 are optional suggestions and need no follow-up push.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🔴 packages/core/modules/requester/requester.js — sweep:_rawRequest(url, options, attempt + 1)\s*; After a rate-limit wait, callers hit a longer or unbounded in-process sleep than the documented cap because retries after that wait drop the accumulated waitedMs back to 0. _rawRequest(url, options, attempt + 1) on the ECONNRESET retry (297), the 5xx ladder retry (392), and the three 401 retries (413, 430, 463) omit the 4th waitedMs argument, defaulting it to 0 instead of forwarding the value carried in from the rate-limit branch. Fix: every retry call site must thread the current waitedMs through so inProcessBudgetMs's maxInProcessWaitMs - waitedMs cap (policy.js) still bounds the total sleep for one logical request when a 5xx/401/ECONNRESET retry is interleaved with rate-limit waits.

  Why this was flagged

  A provider that mixes a 429 (with Retry-After) and an intermittent 500 or 401 in the same call sequence triggers this: the 429 branch accumulates waitedMs (requester.js:366) then a subsequent 5xx retry at line 392 or a 401 retry at 413/430/463 calls _rawRequest(url, options, attempt + 1) with no 4th arg, resetting waitedMs to 0 (default at line 231). The next rate-limit wait then computes its budget as if no time had been slept yet (policy.js inProcessBudgetMs), letting the request sleep well past the documented 5-minute maxInProcessWaitMs cap and the PR's own claim that 'One request may sleep 5 minutes in total.' On the base branch there was no such cap to defeat since waitedMs did not exist; this diff introduces the safeguard and these five call sites silently bypass it.

  Verification: normal. The candidate's factual claim holds. _rawRequest carries a cumulative in-process sleep accumulator (waitedMs, requester.js:231, documented at 227-229 as "Milliseconds this call has slept already ... It counts against the in-process cap"). The two rate-limit retry sites forward it: the backoff-throttle branch passes waitedMs (requester.js:330-335) and the fitting-hinted-wait…

[d-klotz]

Addressed the automated review in 7302a64:

• Reason-only classify on a non-429 (Codex P1): a response that classify() names as a limit but that has no time now follows the same ladder as a bare 429. The last error is a FetchError flagged isRateLimited with the reason, so the queue worker no longer halts and discards it.
• JSON media types (Codex P2, Claude): classify gets the parsed body of any application/json, text/json or +json response, whatever the case. The old prefix match also matched application/jsonp.
• Cumulative wait (Codex P2): waitedMs now travels through every retry (5xx, connection reset, the 401 paths), so a 5xx between two hinted waits no longer resets the 5 minute cap. Two new tests.
• FetchError.create (Graphite): only an undefined body triggers a read, so an empty body the caller passed is not read again.
• SonarCloud: Number.NaN, String.raw, and the throttle handling moved out of _rawRequest into _throttleRetry. The "empty object is useless" note is on { ...(fields || {}) } in invocation-scope.js, which was already there; I only re-indented it.

One thing worth knowing from this round: the Requester now takes an optional random parameter for the jitter, like fetch and backOff. My first tests replaced Math.random, and source-map picks its quicksort pivots with it, so a constant made jest overflow the stack while it formatted a stack trace.

Full packages/core run: 2,692 pass, and the same 63 failures as origin/next in this environment.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, this run also examined two other spots and ruled them out: a classify() hint that names only a reason with no time (requester.js:525) correctly falls back to the fixed backoff ladder rather than retrying unboundedly; and the shared attempt counter across throttle retries and 5xx retries (requester.js:366) reuses the same shared-budget pattern the ladder already had pre-PR, not a new regression.

Extended reasoning...

This run's inline findings (policy.js scope-key stringification and reset-header honouring, and RateLimitError losing statusCode/method/url for non-Requester clients) are new, not previously raised, so a fresh defer note is warranted rather than staying silent. The two items above were checked directly against requester.js's retry/backoff logic and found consistent with pre-existing behavior, so they add no new concern.

[claude]

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, comment @claude review on this pull request, or push a new commit, to trigger a review.

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[seanspeaks]

Adversarial review, run with Claude Code at Sean's request. All 497 of the PR's tests pass; the findings below come from 13 extra adversarial tests (in a scratch worktree, not committed) plus a read of the queue and infrastructure code.

The parsing, RateLimitError, serialisation and redaction work is solid, and most of it can stay as it is. Two things block the merge, and two more should be fixed.

Blocking

1. A non-429 limit that classify recognises without a time now runs the full ~284 s ladder. policy.js:231-255, requester.js:346

• Repro: classify returns { reason: 'daily' } for a 403. The ADR allows a hint that names only a reason. The module has no windows/perMs and no minRetryAfterMs.
• What happens: resolveRateLimitHint falls back to a source: 'backoff' hint, so throttled is true. The unbudgeted ladder then runs on the 403: 7 calls and delays of [1000, 3000, 10000, 30000, 60000, 180000] ms, inside a 29 s deadline. It ends with a plain FetchError that has isRateLimited: true and retryAt: undefined.
• Effect: an HTTP Lambda times out, and a queue worker sleeps about 5 min to call a daily-limited endpoint 7 times. On next the same 403 fails after 1 call.
• Suggested fix: a status other than 429 never uses the ladder. Without a time, throw RateLimitError immediately (retryAt derived from windows.resets when one exists).

2. The scheduling decision lives in the module policy and the Requester.

• Direction from Sean: the decision to wait, requeue or defer must not sit at the API-module level. It should surface as an event, and the worker or orchestrator decides.

• Where the PR puts it: maxInProcessWaitMs is on the module's rateLimit policy. The right value depends on the caller (a 29 s HTTP Lambda vs a 900 s worker that has reservedConcurrency: 20), which a module can't know.

• What a queue worker does today: "requeue" is the halt-rule exemption, then SQS redelivery after the visibility timeout, so retryAt is ignored. With the generated queue config (VisibilityTimeout: 1800, maxReceiveCount: 3, DLQ = InternalErrorQueue; integration-builder.js:570-576):

  • ≤ 5 min: sleeps in process, holding 1 of 20 worker slots for every tenant.
  • 10 min: retries about 20 min late.
  • Daily (Retry-After: 86400): 3 receives about 30 min apart, then the DLQ, where the processor only logs it. The work is lost after about an hour.

• Suggested shape (keeps phase 1 small):

  • The module policy describes the provider only: classify, parsers, minRetryAfterMs, windows, scope. Drop maxInProcessWaitMs.
  • The caller sets the in-process budget through the invocation store: runWithInvocationDeadline({ deadlineAt, maxInProcessWaitMs }). createHandler gives HTTP about 10 s; queue workers get a small default (0 once deferral exists).
  • The Requester stays a signal source: it sleeps within the caller's budget, and otherwise throws RateLimitError.

  Phase 2 then turns the error into a RATE_LIMITED event (details on #655).

Should fix

3. Hinted retries ignore backOff.length. requester.js:487-530

• backOff: [1, 2] with Retry-After: 1 on every response gives 301 calls and about 300 s of sleep before RateLimitError; on next the most is 3 calls.
• It applies to every module (Retry-After is now global). sharepoint and deel set [1, 3], so their authors expect a small number of retries.
• Hinted retries also use up attempt, so a later 5xx in the same request gets no ladder retry.
• Suggested fix: bound hinted attempts, e.g. max(backOff.length, 10) or a separate maxHintedRetries.

4. Token-endpoint limits lose the signal. oauth-2.js:364 (_transportFailureError)

• A classified 403, or a hinted 429, from the token endpoint during a 401 refresh comes back as a plain Error with statusCode, and no isRateLimited/retryAt.
• The queue worker then halts the 403 and discards it, which contradicts the PR's "a 403 limit goes back to SQS".
• Suggested fix: copy isRateLimited, retryAt, waitMs and reason onto the transport error.

Non-blocking

• Entry points with no deadline. admin-scripts admin-script-router/report-router (30 s timeout), script-executor-handler, report-executor-handler, database-migration-handler and workers/db-migration.js don't run under runInvocationScope, so only the 5-min cap applies. A 60 s hint in the 30 s admin router hits the Lambda timeout. With getRemainingTimeInMillis missing, the deadline is Infinity.
• Reserve can be 0. With requestTimeoutMs: 0 (policy.js:280), it slept 9500 of a 9500 ms budget.
• Thundering herd. 50 parallel requests with Retry-After: 10 all retried between 10.0 and 11.0 s (pacing is phase 2, noting it here).
• waitedMs counts only hinted sleeps. A mix of ladder and hinted retries can sleep about 5 + 4.7 min.
• Over-matching classify. If it matches a 404, that becomes a 5-min loop (151 calls). Worth a doc warning.
• HTTP routes. A RateLimitError reaches the client as a generic 500 (app-handler-helpers.js), not a 429/503 with Retry-After.
• Reset headers. One given as a millisecond delta (e.g. 60000) is read as 60000 s.
• Types. RateLimitError is exported at runtime from @friggframework/core but only declared in @friggframework/errors; tsc --strict gives TS2305.
• Verified fine: Retry-After parsing (it rejects -1/NaN/huge values and dates more than 366 days ahead; past dates and 0 become 1 s); classify that throws or returns a Promise is ignored and the headers are used; the body is read once and a 2xx body is never touched; no secrets in message/stack/inspect/JSON.

[cursor]

Frigg 2.0.1 review (Group 2) — COMMENT only

Checklist

• Constructor backOff seconds parameter actually honoured: pass
  • Ladder path still uses this.backOff[attempt] * 1000 for 5xx / backoff-sourced 429s.
  • Header/body hints use Retry-After (etc.); empty backOff: [] forces immediate RateLimitError (tested).
• Requeue decision not in the API module: pass (this PR only classifies/waits/throws; deferral is #657)

Verdict (would): would approve

Findings

1. nit — computeWaitMs floors hinted waits at MIN_HINTED_WAIT_MS (1s).
   • Scenario: Provider returns Retry-After: 0 or a past reset → in-process wait becomes ≥1s instead of 0. Harmless for correctness; document if clients expect a true no-op retry.

No blocking issues. Note for #666: its module-rate-limit-ignored warning remains accurate — runtime reads static rateLimit on the API/Requester class, not Definition.rateLimit / config.rateLimit.

[claude]

⚠️ Code review skipped — your organization's extra usage balance is too low to start another review, so this review was not started.

An organization admin can add extra usage credits at claude.ai/admin-settings/usage.

Once credits are available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

[d-klotz]

Addressed in 3 commits:

1. Non-429 limit without a time (a9fb1cb): throws RateLimitError after 1 call, retryAt/waitMs undefined. No ladder. windows.resets isn't read at runtime yet, so no retryAt from it for now.
2. Caller owns the budget (6eb85ab): maxInProcessWaitMs dropped from static rateLimit. runWithInvocationDeadline({ deadlineAt, maxInProcessWaitMs }, fn), set through createHandler: 10 s HTTP, 60 s queue worker, 300 s outside an invocation. Worker goes to 0 once feat(core): put a rate-limited queue message back at its retry time #657's deferral lands.
3. Hinted retries bounded (468f83a): max 10 per request, and they no longer spend the ladder's attempt. Your backOff: [1, 2] + Retry-After: 1 case is now 11 calls instead of 301.
4. Token endpoint: already in feat(core): put a rate-limited queue message back at its retry time #657. _transportFailureError copies isRateLimited, retryAt, waitMs, reason and module.

Each fix has a repro test that fails on the old head.

Not in this round (non-blocking): admin/db-migration deadlines, reserve of 0, waitedMs counting only hinted sleeps, doc warning on over-matching classify, HTTP 500 vs 429, ms reset headers, TS2305. Thundering herd stays in phase 2.

The RATE_LIMITED event + default handler (your §3/§5 on #655) still needs the ADR update and a rework of #657/#658. #657/#658 already build on this, pushing next.

Can you have another look?

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud