fix(adapter-api): stop the feed reconnecting against a refusal that repeats

[cuibonobo]

Summary

Closes #199.

isFatalFeedError ended the change feed's reconnect loop only for an unrenewable credential (401) and an authorization refusal (403). Its doc comment already stated the general rule — a response that "will reject the next connection identically, so retrying only spins" — but the predicate implemented a narrower version of it.

Every other client-faulting refusal was therefore treated as transient. A GET /changes answered 400 bad_request — which a malformed cursor, kind or include value earns — deserializes to StackQueryError, which is neither class, so subscribeChanges() reported it through onError and retried. reconnectDelay saturates at RECONNECT_MAX_MS, so it settled into an attempt roughly every 15 seconds, indefinitely, sending the same rejected request each time. onReset never fires on that path, so the application had nothing to reconcile from either — it just received the same error on a timer.

The predicate now decides on the wire status, which also keeps it correct as the error taxonomy grows:

const isFatalFeedError = (err: unknown): boolean => {
  if (err instanceof APIAdapterAuthError) return true;
  if (err instanceof StackError) {
    const status = WIRE_ERROR_STATUS[err.code];
    return status >= 400 && status < 500;
  }
  return false;
};

StackError and WIRE_ERROR_STATUS were both already imported in this file, so no new dependency. permission maps to 403, so the explicit StackPermissionError case is subsumed and its import is gone.

The 5xx carve-out is the load-bearing part. WIRE_ERROR_STATUS puts timeout at 503 and migration at 500, and timeout is the answer a server gives while shedding query load. Treating that as fatal would turn a busy server into a permanently dead subscription — worse than the loop this fixes. Enumerating error classes instead of reading the status would be one StackTimeoutError away from exactly that, which is why the status is what the predicate reads.

Spec

Yes — docs/spec/wire-format.md § Backpressure and reconnection gains one paragraph. It described when a client reconnects but never when it gives up, so the 401/403 behavior already shipped was undocumented too; this states the whole rule rather than just the new half.

The status table in the same document already implied it, which is partly why this reads as a fix rather than a new policy:

503 … the request is worth retrying — which is why this is not bad_request: that code tells a client its request was malformed and retrying won't help

Verification

pnpm run format:check, pnpm run lint, pnpm test, pnpm run build, pnpm run typecheck — all clean, in that order. 1320 tests across the workspace; adapter-api goes from 217 to 219.

Both new tests were confirmed to fail against the predicate they pin, in opposite directions:

• against the original 401 || 403 predicate, stops reconnecting after a 4xx the reconnect would only repeat fails;
• against a naive err instanceof StackError predicate with no 5xx carve-out, keeps reconnecting after a 503 the server may recover from fails.

Notes for reviewers

Found while reviewing the change feed in haverstack/server (haverstack/server#94, haverstack/server#95). The server-side question was whether a charset-invalid resume cursor should stay a 400 or become a reset frame. Conclusion was to keep the 400: it isn't a value any conformant server could have minted, so there is nothing to resynchronize from, and a 400 naming the bad value stays diagnosable where a silent reset would let a client persist cursors wrongly and pay a full resync on every reconnect forever. That left this retry loop as the one real cost of the 400, and it belongs here rather than being worked around by making the server quieter.

Worth knowing how narrow the trigger is: this client guards its own cursors at both entry points — opts.since is isValidSeq-checked at subscribe time, and dispatch() adopts a frame id only if (frame.id !== undefined && isValidSeq(frame.id)) — so it will not produce a charset-invalid Last-Event-ID itself. What reaches the loop is an intermediary corrupting the header in transit, a caller that persisted a cursor and mangled it in its own storage, or a future server-side validation this client doesn't anticipate. The fix is cheap and the failure mode is bad, but nobody is hitting this daily.

Note also the pre-existing asymmetry this doesn't change: on a first connection (!settled) any error already rejects subscribeChanges() correctly. Only the reconnect path spun.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FH88yDv3Fk2RmLsU7DU4Rz

---

Generated by Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: ddeaaf4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@haverstack/adapter-api	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR