Make max_retries=0 a supported contract, and let Go's typed operations honour the cap

Merged
JE
2mo ago
+504-40

Closes #718.

A contract decision, not plumbing

The issue asked for a fixture pinning max_retries: 0 ⇒ exactly one request.
Before one could be written, the spec and the SDKs had to be made to agree.
SPEC §2 validation step 4 said max_retries >= 1 → ⊥ BasecampError("usage"),
and documented four divergent outcomes, none of them the spec's.

Three coherent positions existed:

  • A — the spec is right. max_retries means total attempts, so 0
    literally requests that no request be made. Adopting it means Python, Ruby,
    Kotlin and generated Go all start rejecting a value they accept today.
  • B — the SDKs are right. 0 is the natural user spelling of "don't retry";
    four of the five numeric implementations already accept and floor it; and
    §14's own attempt-budget table already blesses "a zero cap" as yielding
    exactly one hop-1 attempt. §2's ⊥ and §14 contradicted each other.
  • C — status quo. Four documented divergences, forever.

This PR adopts B. It resolves an internal SPEC contradiction rather than
deepening one; it matches what four of five numeric implementations already
ship, so it breaks no working code (A would); and the "0 means zero attempts"
objection is an argument for documenting the inherited Ruby name, not for
rejecting the value every user reaches for. Cost: one SDK changes behaviour
(hand-written Go stops panicking and floors instead).

TypeScript and Swift deliberately keep no numeric cap. Their loops are
driven by the per-operation retry.max ceiling, so a client-wide number could
only ever lower the budget, and the only lowering callers want is "off" —
which enableRetry: false already spells. Adding a numeric knob to two SDKs to
express something they can already express is new public API for no new
capability. SPEC now states this rather than leaving it an unexplained gap, and
both runners map a 0 cap onto that knob.

A genuine Go defect, previously unreported

initGeneratedClient constructed the generated client with only
WithHTTPClient and WithRequestEditorFn — never WithRetryConfig. Every
typed operation therefore retried on DefaultRetryConfig{MaxRetries: 3} no
matter what WithMaxRetries(n) said, while that option's doc comment claimed it
"sets the total attempt count for GET requests" and Projects.List is a GET.
A user lowering the cap to fail fast still got three attempts on essentially
every API call; only the raw Get/GetAll and download paths honoured it.

SPEC Appendix F already claimed Go "honors a caller asking for fewer attempts
than an operation declares." It now does. This is the change most likely to be
felt: see MIGRATING.md.

Corrections to the issue's framing

  • "governed vs ungoverned operation" is unmodellable. Every modelled
    operation in behavior-model.json carries a retry block, so "ungoverned"
    cannot mean an operation that lacks one. It means traffic with no operation
    ID at all
    , and DownloadURL is the only conformance-reachable example.
  • configOverrides had no retries key, and no additionalProperties: false
    — so a stray maxRetries would have schema-validated and been silently
    ignored by all six runners: a vacuously green fixture. The harness needed
    changing before the fixture could mean anything.

Verification

Both red proofs captured with REAL_EXIT in the log:

stageresult
un-fixed Goconstruction panics; 0 requests
cap fixed, plumbing absenttyped GetProject makes 3 requests against a cap of 0 — and 3 against a cap of 2
both fixed1 and 2 respectively

Anti-vacuous checks:

  • Override deleted from the fixture → all six runners fail on requestCount
    (exit 2 each). The override is load-bearing everywhere.
  • A misspelled maxRetires now fails conformance-fixtures-check
    (Additional properties are not allowed), where before it validated and was
    ignored six times over.
  • The queued single 503 makes the assertion self-policing: a retrying SDK's
    second request falls off the end of the queue into a 500, so requestCount: 1
    cannot pass vacuously.

Full make check passes clean; all six conformance runners pass.

Honesty about the fixture's strength

Stated in the fixture description, not just here: this is a real regression
guard only in the four SDKs whose loops are pre-check (Go, Python, Ruby,
Swift). Kotlin and TypeScript consult the budget after the first request, so
a 0 and a 1 budget are observably identical there — the same asymmetry #713
hit. The fixture does not claim six-way strength.


Summary by cubic

Accepts max_retries = 0 as “no retries — exactly one attempt” and fixes Go typed operations to honor the caller’s retry cap and base delay. Before: the spec rejected 0, hand‑written Go panicked, generated Go ignored the cap for typed ops (used 3), and its base delay could exceed the backoff ceiling.

  • SPEC: Validation now allows max_retries ≥ 0; every numeric SDK floors 0 to one attempt on every path. The enumeration now includes Ruby among SDKs that reject negative caps. TypeScript/Swift keep no numeric cap; runners map maxRetries to enableRetry with a cutoff of > 1 (0 or 1 disables retry).
  • Go (pkg/basecamp): NewClient now panics only for negative caps (“basecamp: max retries must not be negative”). doRequestURL and download loops floor attempts at 1. initGeneratedClient now passes a RetryConfig so retry‑eligible typed operations honor WithMaxRetries and WithBaseDelay; base delay is clamped to MaxBackoffDelay (30s) and a configured 0 carries through. Non‑idempotent typed operations remain single‑attempt. Tests pin zero‑cap behavior, typed‑op cap plumbing, and base‑delay clamping.
  • Conformance: Adds configOverrides.maxRetries (schema minimum: 0, additionalProperties: false). Plumbed through Go/Python/Ruby/Kotlin; TypeScript/Swift map maxRetries to enableRetry using the same > 1 rule.

Migration

  • Go typed operations may make fewer attempts if you had lowered the cap but previously saw three; raise the cap if you want the old count.
  • If you match on the old panic string, update it to “basecamp: max retries must not be negative”.
  • If you set BaseDelay above 30s, it now clamps to 30s on typed operations; if you set 0, typed operations now use 0s instead of the generated 1s default.