Go: revalidate typed operations' GETs against the ETag cache

Merged
JE
20d ago
+622-95

Part of the API CPU work on the T2 card ("API hosts' CPU has doubled since July").

What

The Go SDK's ETag cache now covers typed operations. The generated client sends through a new etagDoer (go/pkg/basecamp/etag_doer.go), which runs the same algorithm singleRequest runs for the raw path: the same key (URL plus credential), If-None-Match when an entry exists, and a 304 turned back into the cached 200.

Why

WithCache only reached the raw Get/GetAll path and the pages followPagination fetches. Every typed operation sends its first request through the generated client, which used the bare http.Client. So a list call sent If-None-Match from page 2 on but always fetched page 1 in full, and a single-resource Get never revalidated. SPEC §17 applies the cache to every GET; Go was the SDK listed as implementing it.

This matters on the server side: basecamp-cli/* requests were about 18–24% of Rails CPU on the BC5 API hosts (Loki samples on the card), and bc3 now answers an unchanged page with a 304 before rendering it (bc3#13979 for comments, todos, card-table column cards and project recordings; readings follows). A 304 only happens when the client asks.

Details

  • One rule for both paths. conditionalGET decides whether a GET is cached and makes it conditional. singleRequest (raw path) and etagDoer (typed path) both call it.
  • 304 → 200. Generated parsing decodes JSON200 only when the Content-Type says JSON, and a 304 has none, so the synthetic 200 carries application/json. Its other headers are the 304's own. bc3's geared_pagination sets Link and X-Total-Count in an after_action, so they're on its 304s too, and the raw path already reads them from the 304. The tests check that pagination continues and that TotalCount/Truncated stay right after a 304 on page 1.
  • No unanswerable 304s. If-None-Match goes out only when the cached body is there too, and both come from one stored response (Cache.entry). The raw path used to read the body only after the 304 came back.
  • One file per cache entry. Each entry is now responses/<key>.entry, holding the ETag on its first line and the body after it. It's written to a unique temporary file and renamed into place. Before this, ETags lived in etags.json and bodies in responses/<key>.body, published separately. Two writers on one key (two CLI processes sharing the cache directory, or two clients built with CacheEnabled) could leave the newer ETag beside the older body, and that pairing stuck: the server kept answering 304 and the old body was served until the resource changed again. Entries in the old layout read as misses, so the first request after upgrading is a full fetch. Only Clear removes the old layout's files, because an older SDK sharing the directory still uses them. Set now refuses an empty ETag or one containing a line break. On Windows os.Rename isn't promised to be atomic; a rename that fails leaves the previous entry in place.
  • Credential scoping. The key is scoped to the Authorization header, as before. A request without one is no longer cached: an AuthStrategy that authenticates by cookie, API-key header or mTLS gave the key nothing to tell callers apart by.
  • What gets cached: only GET 200s with an ETag and a JSON Content-Type, and only bodies within MaxResponseBodyBytes. A larger body is read only up to that limit, then streamed on uncached.
  • Caller-set validators. A request that already carries If-None-Match passes through untouched.
  • Without WithCache/CacheEnabled, nothing changes.

Other SDKs

None has this gap. TypeScript's cache middleware covers page 1; Swift's HTTPClient covers every page. Ruby, Kotlin, Python and Rust have no ETag cache (SPEC §17 lists that as deferred). TypeScript has the reverse gap: pages 2 and on go through the raw fetchPage closure, which skips the cache middleware (typescript/src/services/base.ts). That's left for a separate change.

Tests

go/pkg/basecamp/etag_doer_test.go:

  • a list revalidates page 1 and page 2, returns the cached cards with the right TotalCount, and replaces the cache after a change;
  • a single-page fetch (Page: 1) revalidates, and Truncated holds after a 304;
  • a single-resource Get revalidates;
  • no If-None-Match is sent without a cache, for another credential's entry, or when the body file is missing;
  • a cookie-authenticated strategy isn't cached;
  • a body over MaxResponseBodyBytes comes back whole and isn't cached;
  • two Cache values on one directory, standing in for two processes, never read or leave a torn ETag/body pair while writers race on one key (cache_test.go also covers a corrupted entry, the old layout reading as a miss and staying put until Clear, refused ETags, and a key too long to prefix a temporary file).

The first three fail on main (no If-None-Match on page 1 or on Get). The cookie test fails without the Authorization check. The torn-pair test fails against the two-file layout, with 1,142 to 1,707 of 2,000 reads torn across runs. make go-check passes (fmt, vet, golangci-lint, tests).

Review before opening for humans

Codex (gpt-6.1-sol, high) raised four findings on the first push. Three are fixed above: the unscoped key for header-less auth, the torn ETag/body read, and the oversized body buffered twice. One is declined in its thread: FromCache in the request hooks, which is the same on the raw path. Its second pass showed that a torn pair across processes persists rather than heals, which overturned my first decline. That's fixed by the one-file layout. Its third pass found the new layout's Set stranding an older SDK's ETag, and temporary names that refused long keys. Both are fixed, and so are three test weaknesses it pointed out.

Release notes: Bug Fixes (bug).