Skip to content

Phase 6a: retry — the policy core and both stacks - #72

Merged
Wahbeh-Mohammad merged 6 commits into
mainfrom
22-phase-6a-retry
Sep 19, 2026
Merged

Wahbeh-Mohammad merged 6 commits into
mainfrom
22-phase-6a-retry

Conversation

@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor

Part of #22. First PR of phase 6a's three-PR stack — the code. Its tests are the next PR up and the phase record the one after. Cut from main at f1fe848 (phase 5 complete); phase 6c (#24) was built in parallel off the same base and lands as its own stack — whichever merges second gets the rebase-and-reprove pass.

What lands

The retry subsystem in dexpace-core — 45 files, +2,165 / −72 — the largest sub-phase in the roadmap: RETRY-1–RETRY-45 and the fifteen RECOV IDs phase 4 handed here (RECOV-17–RECOV-30, RECOV-34), plus four items earlier phases postponed. One policy core, two stacks, no second calculator.

  • Dexpace::Resilience::Policy (extend self) — the tuning constants (200 ms / ×2 / 8 s / 0.2 jitter / 3 sends, RETRY-12), retry_eligible?(status, set:) with RETRY-37's authoritative-contains semantics (a configured set that narrows, narrows), throwable_retryable? as XCUT-6's capability query over Dexpace.each_cause (never is_a?(::IOError) — P6-4's blind spot is stated), backoff_delay (saturating at the cap; a zero initial delay guarded, P6-53), effective_max_retries with RETRY-41's clamp logged, budget_remaining referenced by the recovery engine only (P6-5's text-scannable split), pacing_delay over the private PacingParsers (Retry-After seconds or HTTP-date, retry-after-ms / x-ms-retry-after-ms, X-RateLimit-Reset epoch; every grammar bounded to fifteen digits behind a 64-byte screen, P6-61, so a hostile header answers nil in microseconds rather than stalling), and cancellation? (RETRY-23: a CancelledError anywhere in the chain is never retryable, P6-60).
  • Resend.eligible? — RETRY-5–RETRY-8/RECOV-18 over Method#idempotent? and Body#replayable?. Only .eligible?: 6b's .replayable_body? and NotReplayableError extend this module when 6b lands.
  • RetrySettings — one frozen Data (includes Model), two consumers; max_retries from 5a's Keys::MAX_RETRY_ATTEMPTS with the stage vocabulary fixed (max_attempts = max_retries + 1, P6-6), a caller's status set owned (RECOV-34), random defaulting to the ::Random class (P6-52), logger: for the clamp diagnostic (P6-59).
  • RetryStep / AsyncRetryStep at Stages::RETRY — fork for every drive, the first included, never Cursor#call; close-before-wait with the tracer emission inside the same fence (RETRY-35, R0-4/R1-1); cancellation.check! at the top of every attempt (Clock#sleep(0.0) returns before its token check); the async driver a real trampoline — a private per-call Pump with a running/rearm flag pair (P6-54), measured flat across 2,001 attempts, because the design's recursive sketch overflows at ~1,500 on every supported Ruby; a positive Async.delay with no Fiber.scheduler fails the returned future with SeamError (R2 route 3), a zero-length delay re-arms inline (RETRY-30/31); no scheduler keyword.
  • RecoveryRetry — the recovery-stack engine installed as Recovery::Orchestrator's transport: (P6-3), rescuing ::StandardError and classifying (P6-8; the fatal family passes untouched, RETRY-25), RECOV-20's total-timeout with zero = unbounded and the per-attempt deadline clamped, RECOV-19's re-classification of every re-sent response (503, 503, 200 terminates on the 200), the terminal throwable raised with the trail and cause: nil (P6-9, RETRY-34).
  • OBS-29's per-attempt group emitted by all three drivers through http_tracer_factory: (a callable called once per operation with the cursor, or the request on the recovery stack — P6-7), defaulting to the NULL tracer; retries_exhausted fires only on a retryable failure at a spent budget (P6-58); interface _HTTPTracer declared inside 5c's http_tracer.rbs.
  • Four postponed items landed: the RECOV fifteen (phase 4's), ProtocolError#retryable_by_status? baked at construction from 5a's Retryability and deliberately not #retryable? (4b's; P6-10), CFG-35's throwable half (5a's), the per-attempt half of OBS-29's wiring (5c's). Dexpace::RetryPredicateError flat in error/ (RETRY-40, P6-56).
  • The Cursor context-bundle widening (Task 8, P6-51): Cursor#bundle, bundle: Bundle::NONE on Cursor.build, Pipeline#call, AsyncPipeline#call and both drivers' #advance, carried across #fork; 5b's Step#open_span(request, bundle) and the span correlation now read the cursor's bundle — the first clause of the instrumentation step's precedence rule is live. Transport.conforms? still holds; the departure from 4c's P4-38 reasoning is recorded.
  • Earlier-phase files widened, each with its sig/ mirror: http_date.rb (the day group (\d{1,2}), RETRY-15 — an absent weekday is still refused), error/protocol_error.rb, pipeline/cursor.rb, pipeline.rb, async_pipeline.rb, pipeline/sync_driver.rb, pipeline/async_driver.rb, instrumentation/step.rb, instrumentation/async_step.rb; comments in bundle.rb, tracing.rb, http_tracer.rb. Five earlier-phase test pins the code invalidated ride this branch: dexpace_test.rb's layer table, 4c's cursor_test.rb method-set pin, 5a's http_date_test.rb rejection element, 5b's cursor stand-ins in step_test.rb / async_step_test.rb.
  • sig/ mirrors lib/ one file per file (the two private_constants with hooks.rbs's comment); the manifest regenerated once from 956 to 1005 rows (+49), every row read against the object model and P6-1/P6-2.

Decisions taken in the open, against the plan's text

Ledger rows P6-51–P6-61 and the checklist's "Deviations from the plan" (33 items) itemise them. The plan was written against phase 4/5 designs and one interpreter; the load-bearing corrections: no Step#bundle_for exists (the seam is #open_span), phase 2's FakeTransport was not overwritten, 5c's RecordingHTTPTracer was reused, the async pump is a trampoline not a recursion, module_function is extend self, Random is untyped in RBS (NFR-11).

Layering

Each tip of the stack is green under every gate on its own tree. This branch is green on the SimpleCov floor too — 94.94% on 4.0.6 (94.87% on 3.2.11; 2,051 runs, 0 failures). The tests PR takes the same tree to 99.98%.

Verification

  • Independent review, three rounds by three fresh reviewers with a fix round between each: round 0 2 blocking / 5 should-fix / 3 nits; round 1 1 / 2 / 1; round 2 approve, 0 / 0 / 2 nits. Every finding of each round verified fixed by the next reviewer's own experiment.
  • All seventeen gates individually at this tip on 4.0.6; the matrix set on 3.2.11; honest RuboCop (--ignore-parent-exclusion) 410 files clean.
  • Mutations: 39, 44 and 50 applied by the three reviewers on both 4.0.6 and 3.2.11; every one caught at the final tip except one equivalent (R2-2).
  • By experiment, both interpreters: the calculator's ladder 200 → 6400 ms then the 8 s cap, saturation at attempt 100 without overflow; every pacing form and malformed class; the RETRY-14 convergence test (both stacks exhaust after the same wire sends, and move together when the setting changes); the engine as the orchestrator's transport with the idempotency key stamped once and the same request object re-sent (assert_same); the fork/close/sleep ordering against the real driver; the no-scheduler async wait failing within wall-clock bounds; the seven 6a suites identical under MAX_RETRY_ATTEMPTS unset / 0 / −1 / abc (hermeticity, R0-1).

Known follow-ups from the final review (nits; not blocking a gate)

  • R2-1 — gems/dexpace-core/test/dexpace_test.rb:176: the smoke-suite comment says "five public Resilience constants" beside an assertion listing six.
  • R2-2 — the async pump's close-before-scheduling order in Pump#retry_after is pinned by no test (the reviewer's mutation survives equivalently; the sync side is pinned) — on the tests PR.
  • Findings routed: the design's recursive pump sketch and its RETRY-30 paragraph are false as written → phase 10's inbound list (P6-54); Pipeline.standard must thread settings: and http_tracer_factory: and the async preset's YARD must state the no-scheduler behaviour → 6b's Task 13a; a raising #retryable? propagates unfenced from Policy.throwable_retryable? → phase 9's XCUT-6 disposition.

Add Dexpace::Resilience -- Policy (the two-axis classifier consult, the
backoff calculator, the total pacing-header parser over the private
PacingParsers, the retry-count resolver, the recovery-only budget and
RETRY-12's constants), Resend.eligible? (the re-sendability gate over
phase 1's idempotent set and phase 3b's replayability), and the frozen
RetrySettings both retry stacks build from, the first reader of 5a's
Keys::MAX_RETRY_ATTEMPTS -- plus the flat Dexpace::RetryPredicateError.

Widen two earlier-phase files in place: ProtocolError bakes XCUT-5's
retryable_by_status? from 5a's Retryability at construction (phase 4b's
postponement; deliberately not #retryable?, P6-10), and HTTPDate's day
group becomes (\d{1,2}) so RETRY-15's single-digit day parses through the
one RFC 1123 parser (R1); 5a's rejection loop swaps that element for the
bare-date row CFG-31 still refuses.

One calculator, two budget policies: backoff_delay takes no total-timeout
and budget_remaining is a separate function only the recovery engine
names (R6, P6-5). A zero initial delay answers 0.0 before the power is
taken, because 0.0 * Infinity is NaN and [NaN, 8.0].min raises (P6-53).
Widen Pipeline::Cursor with one read-only member, #bundle -- CTX-14's
per-call correlation bundle -- seeded by a new optional `bundle:` keyword
on Pipeline#call and AsyncPipeline#call (Bundle::NONE when omitted),
validated in Cursor.build, threaded through both private drivers'
#advance and carried across #fork exactly as #options is. No existing
signature moves, and Transport.conforms? still accepts both runtimes
because the keyword is optional beside the SPI's three positionals
(P6-51).

The one consumer is 5b's instrumentation step, on both runtimes:
#open_span now takes the cursor's bundle and prefers its tracer factory
when the bundle is not NONE, else the step's own keyword, and
Tracing.correlate is handed the same bundle, so a populated one pushes
its trace and span ids onto the diagnostic context. That is the
reconciled precedence's first clause, which phase 5 recorded as having
no implementation path; a call that seeds nothing resolves exactly as
phase 5 did. The bundle carries no operation name, so the span is still
named by the method token (P6-52), and it is not the retry step's
HTTP-tracer source (P6-7).

Existing tests only widened where the code required it: the cursor's
public-method pin gains #bundle, and the two step suites' hand-rolled
cursor stand-ins answer #bundle with NONE.
Add Resilience::RetryStep and Resilience::AsyncRetryStep at
Stages::RETRY, over the private RetryStepHelpers mixin that holds the
decision, the delay resolution, the two override hooks and the trail
handling both share. Each drives the downstream chain once per attempt
through a fresh Cursor#fork -- the first drive included -- so hop 1 and
hop n are one object and each attempt re-executes the chain with fresh
per-attempt state (RETRY-44); neither writes cursor state of its own.

The synchronous step waits through 5a's cancellable Clock#sleep and
checks the token at the top of every attempt. The async step is a
trampoline, not a recursion: one Pump per call resumes under a re-arm
flag held by a Thread::Mutex across the flip only, so an inline
settlement re-enters #resume without growing the stack (the design's
recursive sketch overflows at about 1,500 attempts on every supported
Ruby); a positive delay goes through Dexpace::Async.delay, which without
a Fiber.scheduler raises SeamError synchronously and fails the future
with the trail attached (R2's third route), and a zero-length delay
continues inline. A settled fatal answering #retryable? is delivered
unclassified rather than retried (P6-55).

OBS-29's per-attempt group is emitted through the tracer the
`http_tracer_factory:` keyword produces once per operation, called with
the cursor (R3, P6-7); retries_exhausted fires only when a retryable
failure met a spent budget. The interface _HTTPTracer phase 5c declined
to declare arrives in the http_tracer sig with the wiring, `context`
untyped because the recovery stack hands a Request where the stage stack
hands a Cursor. The error-status response is closed before the wait and
before any throwable propagates (RETRY-35); no total-timeout is named on
this stack (RETRY-28, P6-5).
Add Resilience::RecoveryRetry, chapter 9's first stack (RECOV-17 to
RECOV-30 and RECOV-34, which phase 4 postponed), installed as
Recovery::Orchestrator's `transport:` decorator rather than as a
ResponseChain step (P6-3): Transform#apply carries no request to
re-send, and only a transport-decorator position can dispatch its own
re-sends while the request chain's stamping stays once per exchange.
Sitting below the orchestrator's rescue, a raising transport is retried
here as an exception (P6-8), each attempt's response is classified
through phase 4b's own Recovery.buffer_error_body then
ProtocolError.for_or_nil -- the buffering is what frees the connection
before the wait -- and the two terminal shapes are a returned response
when the failure was never retryable and a raised throwable otherwise,
with every prior failure attached as suppressed (P6-9).

This is the only stack with a total-timeout: the budget is a
maximum-attempts cap counting the first send as attempt 1 (max_retries
plus one, an identity, P6-6) and the time remaining through
Policy.budget_remaining, so a delay that would overshoot is suppressed
and the last failure surfaced. The HTTP tracer is produced once per
operation with the Request, there being no cursor on this stack.

The entry file now requires the whole retry layer, the entry test pins
the six Resilience constants and the flat error with the private helpers
unreachable, and the runtime surface manifest gains the forty-eight rows
the phase's public surface adds.
Review round 0 of the phase-6a stack found four behaviours the design
states and the code did not fully honour, and one comment nit.

The pacing parsers converted an unbounded digit run: a 10 MB header
value stalled the retry decision for seconds in String#to_f / #to_i and,
past ~309 digits, emitted Ruby's out-of-range warning that the suite's
NFR-6 raiser turned into an error Policy#parse_form's rescue swallowed.
Both grammars now bound their runs at fifteen digits -- the largest run a
Float carries exactly, and 10**15 seconds is thirty million years, so a
longer run is RETRY-16's out-of-range value and answers nil without a
conversion -- and a 64-byte ceiling in front of every parser keeps the
HTTP-date attempt from reading a hostile value either (the longest
well-formed form is the 29-byte RFC 1123 date).

RetryStep#wait emitted the tracer's attempt_failed outside the RETRY-35
fence, so a tracer that raised left the superseded response open on the
sync driver while the async driver closed it. The emission now runs
inside the same fence as the delay resolution, so the response is
closed before the raise propagates on both drivers.

A caller's should_retry answering true retried a downstream
CancelledError. Policy.cancellation? walks the cause chain for a
CancelledError; Policy.retryable? answers false for one before either
branch, and the stage drivers' decision answers :stop for one before the
re-sendability gate and before the predicate, so RETRY-23's "never" holds
on all three drivers whatever a capability or a predicate says.

A negative configured MAX_RETRY_ATTEMPTS raised at RetrySettings.build,
so RETRY-41's clamp-and-log was unreachable from any driver.
RetrySettings.build takes a logger: keyword, read once at build, and
resolves the configured value through Policy.effective_max_retries, so a
negative configured value is clamped to the default and the clamp logged
as one contained config diagnostic; an explicit negative max_retries: is
still refused by RECOV-34.

Also: step.rb's class comment cited P6-52 for the no-operation-name
statement, which is P6-51's; the RBS mirrors follow each change and the
core manifest gains Policy#cancellation? (1004 -> 1005 rows).
Review round 1 of the phase-6a stack found the sync/async drift round 0
closed for attempt_failed still open on the terminal path: RetryStep's
settle emitted the tracer's retries_exhausted outside any fence, so a
tracer that raised there propagated while the terminal error-status
response it had just been handed stayed open, whereas the async pump's
finish already ran inside its guarded block and closed it.

The trail attachment and the retries_exhausted emission now run inside
the same fence as the decision and the delay resolution, so a throwing
tracer propagates (OBS-30) with the terminal response closed first on
both drivers; a tracer that does not raise leaves the returned response
open exactly as before (RETRY-34), and the exception path is untouched.
The class comment and the method's comment state the terminal path too.
@Wahbeh-Mohammad Wahbeh-Mohammad added type:feature New capability or enhancement area:core Core HTTP, IO, body, context, encoding: HTTP-* IO-* BODY-* CTX-* UTF-* area:resilience Retry, recovery, redirects: RETRY-* RECOV-* REDIR-* labels Sep 18, 2026
@Wahbeh-Mohammad
Wahbeh-Mohammad merged commit e437d11 into main Sep 19, 2026
5 checks passed
@Wahbeh-Mohammad
Wahbeh-Mohammad deleted the 22-phase-6a-retry branch September 19, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Core HTTP, IO, body, context, encoding: HTTP-* IO-* BODY-* CTX-* UTF-* area:resilience Retry, recovery, redirects: RETRY-* RECOV-* REDIR-* type:feature New capability or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant