basecamp / basecamp/basecamp-sdk
Generated Go client's retry loop converts Retry-After to a Duration without clamping
- Dominant language
- Go
- Stars
- 49
- Forks
- 12
- Avg merge
- 20h 47m
- Merged PRs (30d)
- 89
Description
`go/templates/client.tmpl:457` — emitted at `go/pkg/generated/client.gen.go:5810` — does:
```go
if seconds, err := strconv.Atoi(retryAfter); err == nil && seconds > 0 {
retryDelay = time.Duration(seconds) * time.Second
}
```
On a 64-bit build `Retry-After: 9223372036854775807` passes `strconv.Atoi`, and the product wraps to `-1s`. The loop then waits `time.After(retryDelay + jitter)`, which for a negative `retryDelay` is already expired, so a typed operation burns its whole attempt budget back to back against a server that asked it to wait.
This is the same defect two reviewers found on #796 in the hand-written raw path, where it is now fixed: `parseRetryAfter` and `ErrRateLimit` saturate at `math.MaxInt64 / time.Second` (~292 years), a representability bound sanctioned by SPEC §7 note 4 rather than a policy cap (#793). The generated loop keeps a fourth, independent copy of the algorithm and was left alone there because fixing it means a template change plus a regeneration, which #796 deliberately does not do.
Also in the same emitted file, `checkResponse`'s 429 arm (`client.tmpl` ~1328) builds `NewRateLimitError(retryAfter)` from an unchecked `strconv.Atoi`, so the generated error type can carry an over-range value for a caller to multiply.
Related to the two other pre-existing divergences #796 reports in the same loop: it parses delta-seconds only (SPEC §6 step 2's HTTP-date form is dropped, the shape #781 closed in Kotlin and TypeScript), and it adds jitter *on top of* a server-directed delay where the raw path and `downloadURL` replace the curve outright. All three are worth settling in one pass over `client.tmpl`.
Contributor guide
Research direction
Start in go/templates/client.tmpl at the retry loop around line 457 and the 429 handling around line 1328, then compare the raw path fixed in #796 and SPEC §6 and §7 note 4. Regenerate go/pkg/generated/client.gen.go and verify retry-after values cannot overflow, HTTP-date handling and jitter match the intended behavior, and generated 429 errors remain safe for callers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 30/100