Fix SPEC §6's Retry-After parsing in Kotlin and TypeScript, and make the fixture able to fail
SPEC §6's Retry-After parsing algorithm, at the three sites that did not implement it and the one
fixture that could not tell. Go, Python, Ruby and Swift are untouched — they were already compliant.
What changed
Kotlin — kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/Pagination.kt. parseRetryAfter was
toIntOrNull() and nothing else; step 2 is now there. One function, still the only one — all four
consumers (BasecampHttpClient.kt, BaseService.kt, Download.kt twice) already routed through it.
TypeScript — errors.ts, retry.ts, services/base.ts. errors.ts already had all three steps
and kept them module-private while both retry loops re-derived the algorithm with parseInt. The
parser is now exported and both loops call it; the two copies are gone. It is deliberately not
re-exported from index.ts, so the package's public surface is unchanged.
conformance/tests/retry.json. The HTTP-date case is replaced and two rejection cases added.
SPEC.md §6 gains a sentence saying the algorithm defines parsing only, plus the pointer to #775
(below). Appendix D's curated fixture row is updated to match the renamed case.
Kotlin's date parsing: ktor, not a new dependency
This is Kotlin Multiplatform common code, so java.time and SimpleDateFormat are unavailable, and
kotlinx-datetime is not a dependency of the SDK (kotlin/gradle/libs.versions.toml carries
ktor, serialization and coroutines only). Rather than add one or hand-roll a civil-date computation,
step 2 uses ktor's String.fromHttpToGmtDate() and GMTDate() — ktor is already an api dependency
via ktor-client-core, so nothing new enters the dependency graph.
What that parser accepts — corrected in review, having been stated wrongly here twice. It is not
IMF-fixdate only; fromHttpToGmtDate tries a list of patterns. Probed against ktor 3.5.2 rather
than inferred a third time, since its pattern list is a fixed-width mini-language where *** means
exactly three characters:
| spelling | |
|---|---|
Thu, 01 Jan 2060 00:00:00 GMT (IMF-fixdate) | yes |
Thu, 01-Jan-2060 00:00:00 GMT (dashed date) | yes |
Thu 01 Jan 2060 00:00:00 GMT (no comma) | yes |
Thu,01-Jan-2060 00:00:00 GMT (no space after comma) | yes |
Thu, 01-Jan-2060 00-00-00 GMT (dashed time) | yes |
Thu Jan 1 00:00:00 2060 (asctime-like) | yes |
Thursday, 01-Jan-60 00:00:00 GMT (canonical RFC 850) | no |
Thu Jan 1 00:00:00 2060 (canonical asctime) | no |
So Kotlin is neither strict nor RFC-complete: it refuses both canonical obsolete forms while
accepting several shapes no RFC defines. The behaviour is left as it is — narrowing it would move
further from RFC 7231's recipient requirement, not closer, and the breadth is bounded where it
matters, since every pattern is an HTTP-date shape and ISO-8601 or a bare year still falls through.
All eight spellings are pinned in PaginationTest so the next ktor bump is visible rather than
silent, and #775 carries the corrected six-SDK table.
Two hazards the implementation answers explicitly, each with its own test: fromHttpToGmtDate signals
malformed input by throwing, and this sits on the retry path where an escaping exception would
replace a backoff with a crash; and a date centuries out exceeds Int seconds, where a bare toInt()
would hand the caller a negative delay. The remaining seconds are rounded up, matching Swift's
.rounded(.up) and TypeScript's Math.ceil.
Red proof
The old fixture was vacuous — it passed against both defects
conformance/tests/retry.json's "Retry-After HTTP-date format respected" used a past date and
asserted only requestCount: 2 and noError. Against un-fixed code, on origin/main:
$ make conformance-kotlin # Kotlin, integer-only parser
PASS: Retry-After HTTP-date format respected
Passed: 197, Failed: 0, Skipped: 1, Total: 198
REAL_EXIT=0
$ npx vitest run --reporter=verbose -t "Retry-After" # TypeScript, parseInt copy
✓ runner.test.ts > conformance/retry.json > Retry-After HTTP-date format respected 1028ms
Tests 3 passed | 251 skipped (254)
REAL_EXIT=0
The 1028ms is the tell: that is the ~1s exponential backoff, i.e. the date was never honoured, and the
case passed anyway. A parser that ignores dates entirely satisfies both assertions by failing the
integer parse and backing off.
The new fixture fails against the un-fixed TypeScript retry path
$ npx vitest run --reporter=verbose -t "Retry-After"
✓ Retry-After HTTP-date in the past falls through to backoff 1087ms
× Retry-After of 0 is rejected and falls through to backoff
→ expected delay >= 1000ms at gap 0 (allowing 2ms timer slack), got 1.8265000000001237ms
× Negative Retry-After is rejected and falls through to backoff
→ expected delay >= 1000ms at gap 0 (allowing 2ms timer slack), got 1.7471250000007785ms
Tests 2 failed | 3 passed | 251 skipped (256)
REAL_EXIT=1
Retry-After: 0 produced a 1.8ms gap and -5 a 1.7ms gap: the backoff had collapsed
outright, which is what a 0 ms and a −5000 ms sleep look like from the outside. Both are green after
the fix.
What the fixture still cannot see, stated rather than glossed
Run against un-fixed Kotlin, the new fixture is entirely green:
$ make conformance-kotlin # still the integer-only parser
PASS: Retry-After HTTP-date in the past falls through to backoff
PASS: Retry-After of 0 is rejected and falls through to backoff
PASS: Negative Retry-After is rejected and falls through to backoff
Passed: 199, Failed: 0, Skipped: 1, Total: 200
REAL_EXIT=0
That is not an oversight, and it is the one instruction in #564 I did not follow. A future-dated case
asserting delayBetweenRequests cannot be written: a fixture is a static JSON literal and the harness
has no clock, so a date far enough ahead to stay future-dated makes a compliant SDK sleep for the
rest of the decade (Retry-After is exempt from SPEC §7's backoff ceiling, leaving only each
platform's own timer as a bound — and those bounds are absurd rather than helpful here: TypeScript's
is ~24.85 days), while a date near enough to assert a small delay against is past-dated within
seconds of being written. There
is no third static option. No runner has any substitution mechanism for mock response headers, and
five of the six materialize the header map at fixture-load time anyway.
So the positive half of step 2 is pinned by per-SDK unit tests instead, the fixture's
description says so in the file, and #780 tracks giving the harness a clock (it names the
resolve site in each of the six runners, including that Ruby has to move its header map into the
to_return block first). I did not build that mechanism here: it is a six-runner change to the
harness riding inside a two-SDK parsing fix, and make check runs all six suites, so it carries its
own verification surface and deserves its own review.
Kotlin's defect, proved red where it can be
$ ./gradlew :basecamp-sdk:jvmTest --tests "com.basecamp.sdk.PaginationTest"
PaginationTest[jvm] > parseRetryAfterParsesFutureHttpDate()[jvm] FAILED
a future IMF-fixdate must yield a delay ==> expected: not <null>
PaginationTest[jvm] > parseRetryAfterComputesSecondsUntilHttpDate()[jvm] FAILED
actual value is null ==> expected: not <null>
PaginationTest[jvm] > parseRetryAfterSaturatesRatherThanOverflowing()[jvm] FAILED
expected: <2147483647> but was: <null>
REAL_EXIT=1
The other two new Kotlin tests — past date rejected, malformed date rejected — pass before the fix as
well as after. They are guards on the branch being added, not red proofs, and are labelled as such.
After the fix: BUILD SUCCESSFUL, REAL_EXIT=0.
TypeScript's two loops, proved red per site
Run with errors.ts's parser exported but both retry loops still un-fixed, which isolates the
actual defect (the correct parser existed; nothing called it):
× retry-after.test.ts > the shared retry loop honours the parsed value
> waits the seconds remaining until a future HTTP-date
→ expected 1055.463498759916 to be greater than or equal to 179000
× retry-after.test.ts > the shared retry loop honours the parsed value
> falls through to backoff for a zero, a negative and an unparseable value
→ Retry-After: 0: expected 0 to be greater than or equal to 1000
× account-logo.test.ts > rejects Retry-After: 0 on the upload path and backs off instead
→ expected "vi.fn()" to be called with arguments: [ ObjectContaining{…}, …(3) ]
Tests 3 failed | 9 passed (12)
REAL_EXIT=1
1055ms for a three-minute HTTP-date is the missing date branch; 0 for Retry-After: 0 is the
missing > 0 guard. The upload-path case matters on its own because conformance cannot reach
services/base.ts — no fixture drives a multipart upload through a 429 — so that loop's copy of the
bug was invisible to the suite in a second, independent way.
All 12 pass after the fix.
Sources were swapped by cp and restored by cp + diff -q throughout; nothing was git checkout --'d
or stashed.
Two tests were relying on the bug
account-logo.test.ts sent Retry-After: 0 and middleware-lifecycle.test.ts documented the choice
outright — "429 rather than 503: Retry-After is honoured only for 429, so this keeps the loop off the
real exponential-backoff clock". Both were fast because zero collapsed the backoff. Correcting
that costs the suite five real ~1s waits in the lifecycle test (its timeout moves to 20s, the loop
count stays, since cross-talk is what the iteration count is for) and turns the logo test into an
assertion on the delay rather than on the mere fact of a retry.
Review round: step 1 read a prefix, and step 2 had to move with it
Copilot caught parseInt accepting a numeric prefix — 120junk was 120 seconds where the other
five SDKs back off (strconv.Atoi, int(), Integer(exception: false), Int() and toIntOrNull()
all reject a trailing-junk value). TypeScript was the lone outlier in step 1 of the algorithm this PR
exists to make uniform, and the fixture description here cites the 1*DIGIT grammar the code was
contradicting.
Fixing step 1 alone would have made one class of input worse. Date.parse is much wider than RFC
7231 — it reads 2099-01-01, Jan 1 2099, a bare 3000 and even 3000junk as real, future dates —
so 3000junk would have stopped being a 50-minute delay and started being a ~975-year one, newly
falling out of step 1 and into step 2's lap. Both steps now shape-check the whole value before
converting it:
| header | before | after |
|---|---|---|
120junk | 120s | backoff |
2099-01-01 | 2099s | backoff |
3000junk | 3000s (would have become ~975 years) | backoff |
"9"×400 | Infinity → setTimeout clamps to 1ms → tight retry loop | backoff |
+120 | 120s | 120s (unchanged — all five consume a leading sign) |
Mon, 32 Jan 2099 00:00:00 GMT | rejected | rejected (Date.parse NaN still does the calendar) |
The Infinity case was not in the review and falls out of the same gate; it is the worst of the four,
because setTimeout clamps an out-of-range delay to 1ms and turns the longest possible instruction
into a tight loop against a server already answering 429 — the failure SPEC §7's ceiling exists to
prevent, already guarded in oauth/device.ts. Number.isSafeInteger puts TypeScript with Go, Kotlin
and Swift.
The guard did not go far enough: isSafeInteger bounds seconds, not milliseconds
Copilot's second-round finding, and it reached further than reported. Both loops multiply the parsed
seconds by 1000 and hand the result to setTimeout, whose delay is a signed 32-bit int — above
2_147_483_647ms it does not wait longer, it clamps to 1ms. So Retry-After: 2147484, a
perfectly safe integer a hair over 24.85 days, was a 1ms retry against a server asking for a month.
The date branch has it worse, and that is the branch this PR exists to add:
Retry-After: Mon, 01 Jan 2035 ... -> 264130929 s -> 264130929000 ms over 32-bit limit: true
a 264130929000 ms timer actually fired after 2 ms
Any HTTP-date more than ~24.85 days out lands there — one line of a config file away, and far more
reachable than a 2,147,484-second integer.
Both branches now clamp at MAX_RETRY_AFTER_SECONDS = 2_147_483, following oauth/device.ts, which
bounds its own Retry-After at the same number with the reasoning already written down: an
over-ceiling throttle should wait out what it can rather than resending before the server's
throttle. client.ts:98 states the same limit in milliseconds. Chunked scheduling was considered and
declined — its only beneficiary is a wait longer than 24.85 days, a distinction no process that could
observe it will survive. device.ts's two tiers are kept: unrepresentable falls through to
backoff (garbage, not an instruction), representable but unschedulable clamps.
Red proof:
× clamps a representable but unschedulable value to the timer bound → expected 2147484 to be 2147483
× clamps a far-future HTTP-date to the same bound → expected 264130898 to be 2147483
× never chooses a delay the platform would silently collapse to 1ms
→ Retry-After: 2147484: expected 2147484000 to be less than or equal to 2147483647
Tests 3 failed | 11 passed (14) REAL_EXIT=1
The finding also falsified this PR's own prose: the fixture description claimed a far-future date
makes a compliant SDK "sleep for years … nothing clamps it", which was false for TypeScript — it would
have tight-looped. Reworded to name each platform's timer as the real bound.
One parity caveat, stated in the code and tabulated on #775: gating step 2 on IMF-fixdate also stops
TypeScript accepting the obsolete RFC 850 and asctime forms. That matches Ruby and Swift, and leaves
TypeScript stricter than the other three — Go takes all three canonical forms, Python is broadly
lenient, and Kotlin takes a family of variants of its own (see the table above; an earlier revision of
this description called Kotlin strict, which was wrong). Benign — an unrecognized form falls through
to backoff, and BC5 emits IMF-fixdate — but it is the same species of accidental divergence as the
status set, so it belongs with that table rather than in a new issue.
Red proof:
× rejects a value that is only partly numeric → expected 120 to be undefined
× rejects a value too large ... rather than Infinity → expected Infinity to be undefined
× accepts only the IMF-fixdate form → expected 2099 to be undefined
× falls through to backoff for a zero, a negative and an unparseable value
→ Retry-After: 120junk: expected 120000 to be less than or equal to 1100
Tests 4 failed | 13 passed (17) REAL_EXIT=1
The new conformance case does not merely fail against the old parser, it times out — the runner
sat in the 120-second sleep it was handed:
× conformance/retry.json > Partly numeric Retry-After is rejected and falls through to backoff 5003ms
→ Test timed out in 5000ms.
All six runners pass it after the fix, five of them unchanged.
Copilot's second finding — the upload path proven only on the integer branch — is closed by a
future-HTTP-date case through updateAccountLogo. Reverting services/base.ts to an integer-only
parser kills that test and no other (5 survivors, 1 kill), which is exactly the gap described. It
works there and not in the fixture for one reason worth naming: a unit test has a clock and computes
the header at run time. That makes it the cheap half of the coverage #780 tracks.
Migration notes
Both user-observable changes are written up under # Unreleased in MIGRATING.md, since generated
release notes can list what merged but not what a caller has to react to:
- TypeScript —
Retry-Afterparsing is now strict, with a table of the four classes of malformed
header that used to buy a delay and now fall through to backoff. No action needed; the direction
only ever shortens a wait that should not have happened. - Kotlin — the HTTP-date form is now honoured where it was previously ignored. A behaviour gain,
but a caller who compensated for the gap will now double-count the delay until they drop the
workaround.
Out of scope, recorded not changed: which statuses honour Retry-After
The six SDKs sit in three places — Python honours it on any status, Go on 429 and 503, and
Ruby, Kotlin, Swift and TypeScript on 429 alone. That is a live divergence (a 503 carrying
Retry-After is obeyed by two SDKs and ignored by four), but converging it is a behaviour change
across five SDKs and does not belong in a parsing fix. Filed as #775 with the per-SDK table,
re-derived file:line citations, RFC 9110's position and three candidate contracts; SPEC §6 now names
the divergence and points there. The status == 429 condition at all three touched sites is
unchanged, and a new TypeScript test pins it so a future parsing change cannot move it by accident.
Verification
| Gate | Result |
|---|---|
LC_ALL=C make kt-check | REAL_EXIT=0 |
LC_ALL=C make ts-check | REAL_EXIT=0 (87 files / 1657 tests locally) |
LC_ALL=C make doc-constants-check | REAL_EXIT=0 — 27 marked spans across 7 files |
LC_ALL=C.UTF-8 make conformance (all six runners) | REAL_EXIT=0 |
LC_ALL=C make check-fixture-execution | REAL_EXIT=0 |
Locale note: conformance was run under LC_ALL=C.UTF-8, not LC_ALL=C. make conformance cannot
run under LC_ALL=C at all — the Ruby runner reads fixtures unpinned, which is #774 and unrelated
to this change. Flagging it rather than switching quietly; every other gate here ran under LC_ALL=C.
All six conformance runners were exercised even though only two SDKs changed, because the fixture is
shared: the two new rejection cases must hold for Go, Python, Ruby and Swift as well, and they do —
each already rejects 0 and -5 at step 1. No per-SDK skip was needed, and nothing was added to
spec/zero-skip-roster.yml.
Closes #564.