[PARKED] fix(opal-client): honour Retry-After, stop retrying 409, and defer re-fetch after exhausted retries by Zivxx · Pull Request #943 · permitio/opal

Zivxx · 2026-08-17T11:28:04Z

Motivation

This is the client companion of server PR #924, which gave GET /scopes/{id}/policy a classified contract: 503 + Retry-After: 30 (clone in progress), 503 + Retry-After: 5 (clone vanished/corrupt), 409 (branch unresolved, no Retry-After), 404 (path not in repo).

The client understands none of it. PolicyFetcher drives tenacity with only wait + stop and no retry= predicate, so:

a 409 burns the same 5 attempts as a transient 503;
Retry-After is never read — the client backs off on its own schedule (wait_random_exponential(multiplier=1, max=10)), ignoring the server's estimate;
after ~40 s the fetch gives up permanently. PolicyUpdater.update_policy records the error into the store transaction and nothing re-schedules.

A PDP that asks for its bundle while the server is still cloning therefore sits on a stale or empty policy store until the next pub/sub message or WebSocket reconnect — for a low-churn scope, hours.

Against a master server the 409 path is inert (the server does not emit it yet); the 503/Retry-After handling pays off immediately, and the 409 handling lands ahead of #924.

Behaviour

Server answer	Before	After
`503` + `Retry-After: 30`	5 attempts, waits ≤10 s each, then permanent give-up	waits ≥30 s between attempts (whole fetch time-bounded); on exhaustion, a deferred re-fetch
`503` + `Retry-After: 5`	same as above	waits ≥5 s; deferred re-fetch on exhaustion
`503`, no `Retry-After`	same as above	unchanged backoff; deferred re-fetch on exhaustion
`429`	5 attempts, header ignored	retryable, `Retry-After` honoured
`409` branch unresolved	5 attempts over ~40 s	1 attempt, `NonRetryableBundleError`, no re-schedule
`404` path missing	5 attempts, `HTTPException`	1 attempt, still an `HTTPException`
other non-200 / connection error	5 attempts, then give up	unchanged (still retried)
hostile `Retry-After: 99999`	ignored	capped at `POLICY_UPDATER_MAX_RETRY_AFTER` (60 s), per wait and per fetch
policy store unreadable during fetch	exception escaped, no deferral armed	degrades to a full bundle; deferral still arms

Configuration

Three new keys, all backwards-compatible defaults (OPAL_ prefix):

Key	Type	Default	Meaning
`POLICY_UPDATER_MAX_RETRY_AFTER`	float	`60.0`	Bounds server-driven waiting: caps each honoured `Retry-After`, bounds the extra wall-clock time one fetch may spend beyond the operator's own backoff, and ceilings the deferred backoff and flat cadence. `0` = honour no server hint (does not disable retries).
`POLICY_UPDATER_RESCHEDULE_ON_RETRYABLE`	bool	`True`	Master switch for all deferral. `False` restores the previous fire-and-forget behaviour.
`POLICY_UPDATER_MAX_DEFERRED_ROUNDS`	int	`20`	Escalation rounds before the flat cadence.

The whole-fetch time bound is max(POLICY_UPDATER_MAX_RETRY_AFTER, exact worst-case wait of POLICY_UPDATER_CONN_RETRY) — it exists to stop a server-supplied hint from monopolising the serial update queue, never to shorten a retry policy you configured on purpose.

Deferred re-fetch

At most one pending timer at a time. Escalating phase: delay = max(Retry-After, 5·2^(n−1)), clamped to [5 s, MAX_RETRY_AFTER], then jittered into [delay/2, delay] so a recovering fleet does not return in lockstep. That phase lasts 5 + 10 + 20 + 40 + 16×60 = 1035 s before jitter (~8.5–17 min).

After the budget the client does not give up — a client whose WebSocket never reconnects and whose scope never receives a commit would otherwise stay stale forever — it settles into a flat jittered cadence of one request every 30–60 s (mean ~45 s, ~1.3/min per client) until a fetch succeeds. Entering the flat phase logs one warning, not one per round.

Cancelled by a successful fetch, by an incoming policy update, and by stop(). The round counter and flat phase are reset by a successful fetch and by a genuine policy update — deliberately not by a reconnect, since reconnects are frequent during exactly these outages and would make the escalation limit unreachable. The timer re-queues through the existing update queue rather than calling update_policy directly, so it stays serialized with other updates instead of racing a second policy-store transaction.

Logging

Exactly one WARNING per exhausted fetch (classified: retryable / non-retryable / connection error), one per deferral round, and one on entering the flat phase. Per-attempt classification, the 404 "requested paths not found" line, and the "server connection error" line are all debug, so a fleet-wide outage does not multiply log volume by the attempt count.

Tests

90 new tests (36 → 126 in the opal-client suite; full repo 326). No real sleeping except two deliberate real-clock tests for the whole-fetch bound (stop_after_delay is wall-clock based, so a faked sleep would disarm them). Coverage: Retry-After delta / HTTP-date / naive-date / malformed / negative / nan / inf; both caps and the operator-budget floor (exact sum of waits, saturating, no overflow at absurd attempt counts); 409 and 404 single-attempt; 500 and connection errors still retried; 503→200 recovery; non-JSON error bodies; log-level and warning-count contract; jitter envelope and spread; the 5 s floor; reconnect-vs-update round semantics; policy-store read failure; flat-phase engagement, once-only warning and clearing; and an end-to-end pass through the real handle_policy_updates loop.

Each guard names in a comment the single-line mutation it catches. A script applies 34 such mutations and verifies every one fails the suite.

Compatibility

No default behaviour change beyond "stop hammering 409/404", "honour Retry-After", and the new whole-fetch time bound. BundlePathNotFoundError is still a fastapi.HTTPException with status_code=404, so existing 404 handlers keep working.

One visible change: the 404 exception's repr() is now BundlePathNotFoundError(status_code=404, detail=...) rather than fastapi's HTTPException(...), and that string is what lands in the OPA transaction log's error field — anything grepping for HTTPException there needs updating.

Permit's PDP pins opal-client 0.9.6, so shipping this needs a PDP bump; the three keys can then be set per-PDP via pdp_opal_client_config_overrides.

Follow-up (out of scope)

A 404 returned for an unknown base_hash (the server no longer has the delta base) is currently non-retryable and waits for the next update; it could instead be recovered immediately with a forced full re-fetch. Worth a separate PR.

🤖 Generated with Claude Code

…er exhausted retries `PolicyFetcher.fetch_policy_bundle` drove tenacity with only `wait` + `stop` and no `retry=` predicate, so every exception was retried identically: a 409 (the scope's branch cannot be resolved) burned the same five attempts as a transient 503, `Retry-After` was never read, and after ~40s the fetch gave up for good. `PolicyUpdater.update_policy` recorded the error and nothing re-scheduled, so a client that asked for its bundle while the server was still cloning the repo sat on a stale policy store until the next pub/sub message or WebSocket reconnect. The server now answers `GET /scopes/{id}/policy` with a classified contract: 503 + `Retry-After: 30` (clone in progress), 503 + `Retry-After: 5` (clone unavailable), and 409 (branch unresolved, no Retry-After). This teaches the client to read it. - classify responses in `_fetch_policy_bundle`: 503/429 -> `RetryableBundleError` carrying the parsed `Retry-After`; 409 -> `NonRetryableBundleError`; 404 -> `BundlePathNotFoundError`, which subclasses both `NonRetryableBundleError` and fastapi's `HTTPException` so the retry predicate sees one type while callers that caught `HTTPException` keep working. Everything else keeps its current (retryable) behaviour via `throw_if_bad_status_code`. - `parse_retry_after` handles delta-seconds and HTTP-date, clamps past/negative values to 0, and returns None for anything malformed (including "nan"/"inf", which parse as floats but are not usable sleep durations). - `retry=retry_if_not_exception_type(NonRetryableBundleError)` so a 409/404 costs exactly one request. - `wait_retry_after_or_backoff` waits for whichever is longer, the server's hint or the configured backoff, with the hint bounded by the new POLICY_UPDATER_MAX_RETRY_AFTER (default 60s). The cap applies only to the server-supplied value; POLICY_UPDATER_CONN_RETRY keeps its meaning. - `PolicyUpdater` arms at most one deferred re-fetch when a fetch exhausts its retries against a retryable error, cancelled by a successful fetch, by an incoming policy update and by `stop()`, bounded by POLICY_UPDATER_MAX_DEFERRED_ROUNDS (default 20) and gated by POLICY_UPDATER_RESCHEDULE_ON_RETRYABLE (default True). It re-queues through the existing update queue rather than calling `update_policy` directly, so the re-fetch stays serialized with every other update instead of racing a second policy-store transaction. 53 new tests; each new guard carries a comment naming the single-line mutation it catches, and all 16 such mutations were verified to fail the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Adds OPAL_POLICY_UPDATER_MAX_RETRY_AFTER, OPAL_POLICY_UPDATER_RESCHEDULE_ON_RETRYABLE and OPAL_POLICY_UPDATER_MAX_DEFERRED_ROUNDS to the OPAL Client "Policy Updates Configuration" section, next to OPAL_POLICY_UPDATER_CONN_RETRY whose semantics they extend. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…nd budget, store-read failure HIGH-1: the deferred re-fetch had no jitter, so every client that hit the same outage came back on the same tick and re-created the stampede the backoff exists to prevent. The delay is now jittered into [delay/2, delay] via a module-level `_jittered` seam (tests replace it with the identity function). HIGH-2: honouring a large `Retry-After` inside the tenacity loop blocked the serial `_policy_update_queue` for up to attempts x cap — a proxy answering `Retry-After: 300` stalled every other policy update for minutes. The stop condition is now `stop_after_attempt(n) | stop_after_delay(MAX_RETRY_AFTER)`, so one fetch can never hold the queue longer than the cap; the deferred re-fetch, which does not hold the queue, owns the long horizon. MEDIUM-1: `POLICY_UPDATER_MAX_RETRY_AFTER=0` collapsed the deferred backoff to 0 and burned all 20 rounds back-to-back. The delay is now floored at DEFERRED_REFETCH_BASE_SECONDS: `max(min(max(retry_after, backoff), ceiling), 5)`. MEDIUM-2: the round budget reset on every WebSocket reconnect, because `_on_connect` calls `trigger_update_policy` which reset the counter — and reconnects are frequent during exactly the outages the budget is meant to bound, so MAX_DEFERRED_ROUNDS was unreachable. The reset now happens only on a genuine incoming policy update (`_update_policy_callback`) and on a successful fetch. `trigger_update_policy` still cancels the pending timer. MEDIUM-3: `get_policy_version()` ran outside the try in `update_policy`, so when the policy store was restarting alongside the server the exception escaped before the fetch and no deferral was ever armed. It now degrades to a full bundle fetch. MEDIUM-4: added an end-to-end test that drives the deferral through the real `handle_policy_updates` loop (fail 503 -> timer -> queue -> handler -> success). LOW-1: per-attempt classification (and the 404 "requested paths not found" line) dropped to debug; `fetch_policy_bundle` now emits exactly one classified WARNING per exhausted fetch, so a fleet-wide outage no longer multiplies log volume by the attempt count. LOW-2: `stop()` awaits the timer it cancelled, instead of leaving a half-cancelled task behind at shutdown. 17 new tests (36 -> 106 in the opal-client suite). The mutation script now covers 25 single-line mutations across both rounds; all 25 fail the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…he round budget Covers the review's LOW-6/7: MAX_RETRY_AFTER also bounds a whole fetch and ceilings the deferred backoff; 429 is retryable alongside 503; the deferred sequence is 5, 10, 20, 40, 60s jittered into [delay/2, delay]; the round budget is not reset by a reconnect, and the resulting worst case is stated explicitly (1035s of deferred waiting before jitter, i.e. ~8.5-17 minutes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ate the operator's retry budget NEW-1: the deferred round budget was terminal — a client whose WebSocket never reconnects and whose scope never receives a commit stayed permanently stale after a long clone. The budget now bounds ESCALATION only: past POLICY_UPDATER_MAX_DEFERRED_ROUNDS the client keeps deferring at a flat `_jittered(POLICY_UPDATER_MAX_RETRY_AFTER)` cadence (30-60s at defaults, floored at 5s) until a fetch succeeds or a genuine policy update arrives. Entering the flat phase logs one warning, not one per round. POLICY_UPDATER_RESCHEDULE_ON_RETRYABLE=false still disables all deferral, including the flat phase. NEW-2: `stop_after_delay(cap)` silently truncated an operator's own POLICY_UPDATER_CONN_RETRY — with `wait_fixed(0.2) x 5` and a 0.3s cap, only 2 of the 5 configured attempts ran. The bound is now `max(cap, POLICY_UPDATER_CONN_RETRY.worstCaseTotalWait())`, a new method on ConnRetryOptions that uses max_wait for the exponential strategies and wait_time for fixed. The whole-fetch bound exists to stop a *server-supplied* hint from monopolising the serial update queue, not to shorten a deliberate configuration. NEW-4: the per-attempt "server connection error" line drops to debug, so a connection-refused outage also costs exactly one WARNING per exhausted fetch rather than one per attempt. NEW-5: as a consequence of NEW-2, POLICY_UPDATER_MAX_RETRY_AFTER=0 now means "honour no server hint" rather than "no retries" — pinned by a test. LOW-3: the status-classification comment now names its provenance (opal-server PR #924, scopes API) and notes that only the 503 path is exercised on older servers. Two existing tests were rewritten rather than deleted, because this changes their behaviour on purpose: `test_deferred_rounds_are_bounded` becomes `test_the_escalation_stops_at_the_round_budget`, and the stop-condition test now pins the max() semantics. The reconnect test pins OPAL_STATISTICS_ENABLED off so it does not depend on ambient config. 16 new tests (106 -> 122 in the opal-client suite). The mutation script now covers 31 single-line mutations across all three rounds; all 31 fail the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… the flat cadence NEW-3: POLICY_UPDATER_MAX_RETRY_AFTER now describes the same three roles in config.py and configuration.mdx (caps each honoured Retry-After; bounds the extra wall-clock time one fetch may spend beyond the operator's own backoff; ceilings the deferred backoff and flat cadence). The "does not clamp POLICY_UPDATER_CONN_RETRY" clause is gone from both — it contradicted the whole-fetch bound before NEW-2 and is redundant after it. NEW-1/NEW-5 docs: MAX_DEFERRED_ROUNDS is described as escalation rounds before the flat cadence, the "gives up / waits for a reconnect" wording is gone, and the steady-state rate is stated (one request every 30-60s per client, mean ~45s, i.e. about 1.3 per minute). The "setting it to 0 disables in-fetch waiting" sentence is corrected: 0 means "honour no server hint" and does not disable retries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… not attempts x max_wait NEW-6: `ConnRetryOptions.worstCaseTotalWait()` estimated the budget as `attempts x max_wait`. `max_wait` defaults to tenacity's MAX_WAIT (~4.6e18), so an operator setting `{"wait_strategy": "exponential", "wait_time": 1, "attempts": 5}` without pinning max_wait produced a budget of ~2.3e19 — and since the whole-fetch bound is `max(cap, budget)`, the HIGH-2 protection that keeps one fetch from monopolising the serial update queue silently disappeared. Now the exact sum: `attempts` attempts are separated by `attempts - 1` waits; exponential/random_exponential sum `min(wait_time * 2**i, max_wait)` over those gaps, fixed uses `n_waits * wait_time`. The example above is 1+2+4+8 = 15s. Implemented with a saturation break rather than the literal generator expression: once the doubling reaches `max_wait` every remaining wait is `max_wait`, so they are added in one step. That is not only cheaper — it is required, because `wait_time * 2 ** i` raises OverflowError once i passes 1023 (a float base overflows there rather than saturating to inf), which a large `attempts` would otherwise hit at client startup. Covered by a 3000-attempt regression test. At the shipped defaults the budget drops from 50.0 to 15.0 and the effective bound is unchanged at max(60, 15) = 60. 4 new tests (122 -> 126 in the opal-client suite). The mutation script gains four NEW-6 guards (coarse estimate, attempts-vs-gaps off-by-one, fixed-strategy gap count, missing saturation break); all 34 mutations across the four rounds fail the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

netlify · 2026-08-17T11:28:11Z

✅ Deploy Preview for opal-docs ready!

Name	Link
🔨 Latest commit	`d3fa629`
🔍 Latest deploy log	https://app.netlify.com/projects/opal-docs/deploys/6a82f0476a5a1800086e8f90
😎 Deploy Preview	https://deploy-preview-943--opal-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Zivxx · 2026-08-17T12:39:01Z

Parked as draft: we are taking the server-side approach in #924 (publish a policy notification when a clone becomes ready, and hold GET /scopes/{id}/policy for a bounded time while a clone is in progress), so no client change is required for the PR3 rollout. Keeping this open as reviewed hardening for later.

Zivxx and others added 7 commits August 17, 2026 13:18

Zivxx marked this pull request as draft August 17, 2026 12:38

Zivxx changed the title ~~fix(opal-client): honour Retry-After, stop retrying 409, and defer re-fetch after exhausted retries~~ [PARKED] fix(opal-client): honour Retry-After, stop retrying 409, and defer re-fetch after exhausted retries Aug 17, 2026

Zivxx closed this Aug 18, 2026

Navigation Menu

Uh oh!

[PARKED] fix(opal-client): honour Retry-After, stop retrying 409, and defer re-fetch after exhausted retries - #943

[PARKED] fix(opal-client): honour Retry-After, stop retrying 409, and defer re-fetch after exhausted retries#943
Zivxx wants to merge 7 commits into
masterfrom
ziv/opal-client-honour-retry-after

Zivxx commented Aug 17, 2026

Uh oh!

netlify Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

Zivxx commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Zivxx commented Aug 17, 2026

Motivation

Behaviour

Configuration

Deferred re-fetch

Logging

Tests

Compatibility

Follow-up (out of scope)

Uh oh!

netlify Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for opal-docs ready!

Uh oh!

Zivxx commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

netlify Bot commented Aug 17, 2026 •

edited

Loading