Skip to content

Commit 5a9dac0

Browse files
fix(server): a supervised stop exits 0 instead of failing (cnighswonger#277)
server.close() waits for in-flight requests, and a live session always has one (the streaming /v1/messages response) — so the 5s watchdog was the normal exit under a supervisor, not the exception. Exiting 1 there made every systemctl stop log status=1/FAILURE and tripped Restart=on-failure on a deliberate stop. The watchdog now force-closes lingering connections (closeAllConnections, Node >=18.2; feature-detected, prior behavior on the 18.0/18.1 floor), reports the forcing on stderr, and exits 0. Verified locally merged onto main with cnighswonger#274: full suite 1437/0. Closes cnighswonger#277
1 parent 540981a commit 5a9dac0

2 files changed

Lines changed: 100 additions & 1 deletion

File tree

proxy/server.mjs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,13 +640,31 @@ if (invokedAsScript) {
640640
process.exit(1);
641641
});
642642

643+
// A supervised stop is a SUCCESS, however it ends. server.close() waits for
644+
// in-flight requests, and a live Claude Code session always has one (the
645+
// streaming /v1/messages response), so the graceful path alone never
646+
// resolves — the watchdog is the normal exit under systemd, not the
647+
// exception. Exiting 1 there made every `systemctl stop` log
648+
// "status=1/FAILURE", which (a) makes a crash and a clean stop
649+
// indistinguishable in the journal and (b) trips Restart=on-failure on a
650+
// deliberate stop. Force the laggards, report the forcing on stderr, exit 0.
643651
const shutdown = () => {
644652
if (!active) {
645653
process.exit(0);
646654
return;
647655
}
648656
active.close().finally(() => process.exit(0));
649-
setTimeout(() => process.exit(1), 5000).unref();
657+
setTimeout(() => {
658+
process.stderr.write(
659+
"[cache-fix] shutdown: in-flight connections still open after 5s — forcing close\n",
660+
);
661+
// Node >=18.2; package.json engines allows 18.0/18.1, where the
662+
// pre-existing behavior (exit without forcing) is the only option.
663+
if (typeof active.server.closeAllConnections === "function") {
664+
active.server.closeAllConnections();
665+
}
666+
process.exit(0);
667+
}, 5000).unref();
650668
};
651669
process.on("SIGTERM", shutdown);
652670
process.on("SIGINT", shutdown);

test/shutdown-exit-code.test.mjs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { describe, it } from "node:test";
2+
import assert from "node:assert/strict";
3+
import net from "node:net";
4+
import { spawn } from "node:child_process";
5+
6+
// A supervised stop must exit 0 whichever path it takes. server.close() waits
7+
// for in-flight requests, and a live session always has one (the streaming
8+
// /v1/messages response), so the 5s watchdog is the NORMAL exit under systemd.
9+
// It used to exit(1) there, which made `systemctl stop` log status=1/FAILURE —
10+
// a clean stop and a crash became indistinguishable, and Restart=on-failure
11+
// fired on deliberate stops.
12+
13+
function startProxy() {
14+
const proc = spawn(process.execPath, ["proxy/server.mjs"], {
15+
env: { ...process.env, CACHE_FIX_PROXY_PORT: "0" },
16+
stdio: ["pipe", "pipe", "pipe"],
17+
});
18+
const port = new Promise((resolve, reject) => {
19+
let out = "";
20+
proc.stdout.on("data", (c) => {
21+
out += c.toString();
22+
const m = out.match(/listening on [\d.]+:(\d+)/);
23+
if (m) resolve(parseInt(m[1], 10));
24+
});
25+
proc.on("exit", (code) => reject(new Error(`Proxy exited ${code}`)));
26+
setTimeout(() => reject(new Error("Proxy start timeout")), 5000);
27+
});
28+
let stderr = "";
29+
proc.stderr.on("data", (c) => (stderr += c.toString()));
30+
return { proc, port, stderr: () => stderr };
31+
}
32+
33+
function exitOf(proc) {
34+
return new Promise((resolve) => {
35+
proc.on("exit", (code, signal) => resolve({ code, signal }));
36+
});
37+
}
38+
39+
describe("SIGTERM exit code", () => {
40+
it("exits 0 when nothing is in flight", async () => {
41+
const { proc, port } = startProxy();
42+
await port;
43+
const exited = exitOf(proc);
44+
proc.kill("SIGTERM");
45+
const { code } = await exited;
46+
assert.equal(code, 0, "clean shutdown must exit 0");
47+
});
48+
49+
it("exits 0 via the watchdog when a request is still in flight", async () => {
50+
const { proc, port, stderr } = startProxy();
51+
const p = await port;
52+
53+
// Announce a body we never finish sending: the request stays in flight,
54+
// so server.close() cannot resolve and the watchdog path is taken.
55+
const sock = net.createConnection(p, "127.0.0.1");
56+
await new Promise((resolve) => sock.on("connect", resolve));
57+
sock.write(
58+
"POST /v1/messages HTTP/1.1\r\nHost: 127.0.0.1\r\n" +
59+
"Content-Length: 5000\r\n\r\npartial",
60+
);
61+
await new Promise((r) => setTimeout(r, 300));
62+
63+
const exited = exitOf(proc);
64+
const started = Date.now();
65+
proc.kill("SIGTERM");
66+
const { code } = await exited;
67+
const elapsed = Date.now() - started;
68+
69+
assert.equal(code, 0, "watchdog shutdown must exit 0, not 1");
70+
assert.ok(
71+
elapsed >= 4500,
72+
`expected the 5s watchdog path, exited after ${elapsed}ms`,
73+
);
74+
assert.match(
75+
stderr(),
76+
/forcing close/,
77+
"the forced path must stay visible on stderr",
78+
);
79+
sock.destroy();
80+
});
81+
});

0 commit comments

Comments
 (0)