Change-feed client reconnects forever on a 4xx the server will refuse identically

[cuibonobo]

isFatalFeedError in packages/adapter-api/src/index.ts covers only 401 and 403. Every other client-side refusal is treated as transient, so subscribeChanges() reconnects against it indefinitely.

const isFatalFeedError = (err: unknown): boolean =>
  err instanceof APIAdapterAuthError || err instanceof StackPermissionError;

Its own doc comment already states the right rule — "will reject the next connection identically, so retrying only spins" — but the predicate implements a narrower version of it than the rule warrants.

The trace

readFeed() throws errorForResponse() on any non-ok response, and errorForResponse() runs deserializeError(body), so a 400 bad_request comes back as a StackQueryError. That's neither APIAdapterAuthError nor StackPermissionError, so the pump reports it through onError and loops:

opts.onError?.(err);
if (isFatalFeedError(err)) return;   // false for StackQueryError
...
await sleep(reconnectDelay(attempt++));

reconnectDelay saturates at RECONNECT_MAX_MS = 30_000 with full jitter, so this settles into a retry roughly every 15s, forever, re-sending the same rejected request. onReset never fires, so the application never reconciles either — it just receives the same error on a timer.

Note the asymmetry: on the first connection (!settled) the same error correctly rejects subscribeChanges(). Only the reconnect path spins.

How it's reached

Narrowly, but really. A GET /changes request can earn a 400 from an invalid kind, an invalid include, or a resume cursor outside the isValidSeq charset.

This client guards its own cursors at both entry points — opts.since is 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's left 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. In each case the retry is guaranteed to fail the same way.

Suggested fix

Decide on the wire status rather than by enumerating classes, which also keeps the predicate 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 is already imported from @haverstack/core in this file and WIRE_ERROR_STATUS from @haverstack/wire-types, so this needs no new dependency.

The thing to be careful about: two codes in WIRE_ERROR_STATUS are 5xx and must stay retryable — timeout: 503 and migration: 500. A status-based predicate gets both right; an enumerate-the-classes version is one StackTimeoutError away from turning a shed-load response into a permanently dead subscription. permission: 403 is covered by the same branch, so the explicit StackPermissionError case can go.

Worth a brief note in the doc comment that the 5xx codes are deliberately excluded, since that's the part a future edit could get wrong.

Test

packages/adapter-api/tests/change-feed.test.ts already has "stops reconnecting after a fatal auth failure" under describe('reconnection'), which is the shape to mirror: connect, let the reconnect come back 400 bad_request, assert the fetch count holds after more time passes. A companion asserting that a 503 timeout does keep reconnecting would pin the boundary from the other side — that's the direction a regression would go.

Context

Came out of a security and cohesiveness review of the change feed in haverstack/server (haverstack/server#94, haverstack/server#95). The server-side question was whether a charset-invalid 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 is diagnosable where a silent reset would let a client persist cursors wrongly and pay a full resync on every reconnect indefinitely. That leaves the retry loop as the one genuine cost of the 400, and it belongs here rather than being worked around by making the server quieter.