live() treats every 4xx reconnect as permanent, including 408 and 429

[frenzzy]

Describe the bug

live()'s reconnect loop treats the whole 4xx band as permanent, so a rate-limited reconnect ends the subscription for good. A 429 is the one status that explicitly means retry later — and the Retry-After that comes with it is never read.

The rule is stated in the source (packages/web/server-functions/src/client.ts):

// Definite rejections fail fast: a 4xx means the server
// understood and refused — auth revoked, resource gone —
// and retrying cannot change the answer.
if (
  error !== null &&
  typeof error === "object" &&
  typeof error.status === "number" &&
  error.status >= 400 &&
  error.status < 500
) {
  stopped = true;
  emitClosed(error);
  throw error;
}

That reasoning holds for 401, 403, 404. It is exactly wrong for 408 and 429, which say the opposite: the request may be repeated.

It also matters where these come from. A 403 is almost always the application's own answer; a 429 is very often not — it is a CDN, an API gateway or a rate limiter in front of the app, shedding a burst it fully intends to serve a second later. So the statuses most likely to be transient, and least likely to have come from the application at all, are the ones that permanently close the stream.

The blast radius is bigger than one failed call: a live source that has been running fine is torn down and never re-established. Everything downstream latches on its last value with no way to learn that updates stopped, unless it subscribed to onstatus.

Steps to reproduce

mkdir sf-live && cd sf-live && npm init -y
npm i @solidjs/web@2.0.0-rc.4
node repro.mjs

repro.mjs:

import { AsyncLocalStorage } from "node:async_hooks";
globalThis[Symbol.for("solid.RequestContext")] = new AsyncLocalStorage();
const srv = await import("@solidjs/web/server-functions/server");
const cli = await import("@solidjs/web/server-functions/client");

// a live source: yields once, then the connection dies
srv.registerServerFunction("ticker", async function* () {
  yield { tick: 1 };
  throw new Error("connection lost");
});

async function run(reconnectStatus) {
  let calls = 0;
  globalThis.fetch = (address, init) => {
    calls++;
    if (calls === 1) {
      const request = new Request(new URL(address.toString(), "http://localhost"), init);
      request.headers.set("Sec-Fetch-Site", "same-origin");
      return srv.handleServerFunctionRequest(request);
    }
    if (calls > 3) return new Response(null, { status: 204 }); // let a retrying loop finish
    return new Response("nope", { status: reconnectStatus, headers: { "retry-after": "1" } });
  };

  const iterable = cli.live(cli.createServerReference("ticker"))();
  let outcome = "kept retrying";
  try {
    for await (const value of iterable) void value;
  } catch (error) {
    outcome = `gave up (threw status ${error?.status})`;
  }
  console.log(
    `reconnect answered ${reconnectStatus}`.padEnd(26),
    "->", outcome.padEnd(30), `after ${calls} fetch attempts`);
}

for (const status of [403, 408, 429, 503]) await run(status);

Output on 2.0.0-rc.4 and on next (ec523607):

reconnect answered 403     -> gave up (threw status 403)     after 2 fetch attempts
reconnect answered 408     -> gave up (threw status 408)     after 2 fetch attempts
reconnect answered 429     -> gave up (threw status 429)     after 2 fetch attempts
reconnect answered 503     -> kept retrying                  after 4 fetch attempts

403 and 503 are classified correctly. 408 and 429 are on the wrong side of a line drawn at 500 instead of at retryability, and the Retry-After: 1 sitting on those responses is ignored.

What the statuses actually say

• 429 (RFC 6585 §4): "The 429 status code indicates that the user has sent too many requests in a given amount of time ("rate limiting")", and the response "MAY include a Retry-After header indicating how long to wait before making a new request." The status exists to say when to come back.
• 408 (RFC 9110 §15.5.9): "the server did not receive a complete request message within the time that it was prepared to wait", and "If the client has an outstanding request in transit, it MAY repeat that request."

Neither is a refusal the server stands behind, which is what the code's comment assumes of the band.

Worth being precise about the practice, since I nearly got it wrong myself: no RFC says "408/429 are retryable and other 4xx are not", and Envoy's retriable-4xx policy covers 409 only. What Envoy does instead is refuse to hardcode the question — retryability is configured per status through retriable-status-codes, plus a dedicated envoy-ratelimited policy for the rate-limit case. So the practice is not "everyone retries 429 automatically"; it is that mature retry layers treat the retryable set as a decision, and none of them draw it at "below 500".

Expected behavior

A reconnect gives up when repeating the request cannot help. 408 and 429 are the cases where it explicitly can.

Options

1. Exclude the retryable statuses from the definite set — 408, 429, and arguably 425 Too Early. Two lines, no API change, and it moves the line from "below 500" to what the statuses actually mean. My preference.
2. Honour Retry-After when it is there. Strictly better than backoff for 429 and 503 alike, since the server is telling you the answer. Needs the header to survive to this layer — today the classifier sees only a stamped error.status, so this is a slightly bigger change than (1) and could follow it.
3. Invert the test: retry everything except a known-permanent list (400, 401, 403, 404, 405, 410, 422). Safer for statuses nobody thought about, at the cost of retrying genuine authoring mistakes forever.
4. Make it policy. Expose the classifier so an application can decide — flexible, and it means every app now owns a decision that has one right default.

(1) alone fixes the reported behaviour; (2) is the natural follow-up if you want the retry to be well-mannered rather than merely persistent.

Related

Came out of the transport review in #3088, which drew the same 5xx/4xx line for a different purpose. #3097 argues that the status should not be the failure signal at all for responses this protocol produced — this issue is about a status that genuinely did come from HTTP infrastructure, so the two do not collide.

[ryansolid]

Fixed on next in 02f87fe (options 1 + 2). The definite-rejection line moved from "below 500" to what the statuses say: 408, 425 and 429 reconnect like a 5xx, and a Retry-After on any status is the peer inviting the retry — a 403 carrying one retries too. The named wait replaces the exponential backoff guess for that attempt, capped at 60s (retrying a shade early against a misconfigured header costs one more named wait; sitting out an unbounded one would end the stream in all but name), and it rides the surfaced error as retryAfter (seconds, HTTP-date form converted) for policy layers. Spec: packages/web/test/server/server-functions-live-retry.spec.tsx.

[frenzzy]

Verified on cc2fb53e. One note for anyone re-running the repro above, because it briefly fooled me: it puts Retry-After: 1 on every synthetic response, so under the new rule all four statuses retry — including the 403, which is the "a Retry-After is the peer inviting the retry" clause working as designed, not a regression.

Dropping that header gives the clean picture:

403 -> gave up (threw status 403)   after 2 fetch attempts
408 -> kept retrying                after 4 fetch attempts
429 -> kept retrying                after 4 fetch attempts
503 -> kept retrying                after 4 fetch attempts

Exactly the line I was arguing for, and the retryAfter on the surfaced error is a better answer than option (2) as I wrote it — I had assumed the header would be awkward to get down to that layer.