Skip to content

Commit 99e1fa2

Browse files
authored
Merge pull request #57 from christianlappin/fix/gateway-heartbeat-readystate-guard
fix(gateway): guard ws.send with readyState check to prevent heartbeat-after-close crash
2 parents ae0ad9c + 8e7e7b2 commit 99e1fa2

2 files changed

Lines changed: 156 additions & 6 deletions

File tree

src/gateway.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,25 @@ export async function connectGateway(
128128
return DEFAULT_RECONNECT_MS;
129129
}
130130

131+
// ws.send throws InvalidStateError ("Sent before connected") if the socket
132+
// is CONNECTING (0), CLOSING (2), or CLOSED (3). The heartbeat timer can
133+
// outlive the socket it was scheduled against (e.g. after onclose fires
134+
// between the interval being set and the next tick), so guard every send.
135+
// Returns true if the frame was actually sent.
136+
function safeSend(payload: object): boolean {
137+
if (!ws || ws.readyState !== WebSocket.OPEN) return false;
138+
try {
139+
ws.send(JSON.stringify(payload));
140+
return true;
141+
} catch (error) {
142+
ctx.logger.warn("Gateway ws.send failed", {
143+
readyState: ws.readyState,
144+
error: error instanceof Error ? error.message : String(error),
145+
});
146+
return false;
147+
}
148+
}
149+
131150
function connect(url: string, resume: boolean) {
132151
if (closed) return;
133152

@@ -154,12 +173,12 @@ export async function connectGateway(
154173
startHeartbeat(heartbeatMs);
155174

156175
if (resume && sessionId) {
157-
ws?.send(JSON.stringify({
176+
safeSend({
158177
op: 6,
159178
d: { token: `Bot ${token}`, session_id: sessionId, seq: sequence },
160-
}));
179+
});
161180
} else {
162-
ws?.send(JSON.stringify({
181+
safeSend({
163182
op: 2,
164183
d: {
165184
token: `Bot ${token}`,
@@ -170,7 +189,7 @@ export async function connectGateway(
170189
device: "paperclip-plugin-discord",
171190
},
172191
},
173-
}));
192+
});
174193
}
175194
break;
176195
}
@@ -215,7 +234,7 @@ export async function connectGateway(
215234
}
216235

217236
case 1: {
218-
ws?.send(JSON.stringify({ op: 1, d: sequence }));
237+
safeSend({ op: 1, d: sequence });
219238
break;
220239
}
221240

@@ -281,7 +300,13 @@ export async function connectGateway(
281300
if (heartbeatAckTimeout) clearTimeout(heartbeatAckTimeout);
282301

283302
const sendHeartbeat = () => {
284-
ws?.send(JSON.stringify({ op: 1, d: sequence }));
303+
// If the socket isn't OPEN, skip this tick entirely. Don't schedule an
304+
// ack timeout for a frame we never sent — onclose will trigger a
305+
// reconnect, which is the correct recovery path. Sending here without
306+
// the guard is what caused the long-lived `InvalidStateError: Sent
307+
// before connected` crash that took the worker down on 2026-05-12.
308+
const sent = safeSend({ op: 1, d: sequence });
309+
if (!sent) return;
285310
heartbeatAckTimeout = setTimeout(() => {
286311
ctx.logger.warn("Heartbeat ACK not received, forcing reconnect");
287312
cleanup();

tests/gateway.test.ts

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,131 @@ describe("connectGateway", () => {
4646
result.close(); // should not throw
4747
});
4848

49+
it("does not throw 'Sent before connected' when a heartbeat tick fires on a CLOSED socket (2026-05-12 crash regression)", async () => {
50+
// Reproduce the InvalidStateError that crashed the worker for two weeks:
51+
// a heartbeat scheduled via setInterval fired between onclose firing and
52+
// the interval being cleared, calling ws.send on a CLOSED socket.
53+
//
54+
// Test must fail without the safeSend readyState guard. Two timing
55+
// constraints make this deterministic and prevent the ACK-timeout
56+
// teardown from clearing the interval BEFORE the regression-tick fires
57+
// (which previously made the test pass for the wrong reason):
58+
//
59+
// 1. Math.random is mocked to 0 so jitter is zero — first heartbeat
60+
// sends at t=0 instead of at jitter ms.
61+
// 2. heartbeat_interval is large (10_000ms) so the ACK timeout from
62+
// the first send is scheduled for t=20_000ms, well after the
63+
// regression-exercising tick at t=10_000ms.
64+
vi.useFakeTimers();
65+
const randomSpy = vi.spyOn(Math, "random").mockReturnValue(0);
66+
67+
class FlakyFakeWebSocket {
68+
static OPEN = 1;
69+
static CLOSING = 2;
70+
static CLOSED = 3;
71+
static instances: FlakyFakeWebSocket[] = [];
72+
73+
onopen: (() => void) | null = null;
74+
onmessage: ((event: { data: string }) => void) | null = null;
75+
onclose: ((event: { code: number; reason: string }) => void) | null = null;
76+
onerror: (() => void) | null = null;
77+
sent: string[] = [];
78+
readyState = 1; // OPEN
79+
80+
constructor(_url: string) {
81+
FlakyFakeWebSocket.instances.push(this);
82+
}
83+
84+
send(payload: string) {
85+
if (this.readyState !== 1) {
86+
// Mirror the real WHATWG WebSocket behaviour.
87+
throw new DOMException("Sent before connected.", "InvalidStateError");
88+
}
89+
this.sent.push(payload);
90+
}
91+
92+
close() {
93+
this.readyState = 3;
94+
}
95+
}
96+
97+
// Expose OPEN/CLOSED on the static `WebSocket` reference the source reads.
98+
globalThis.WebSocket = FlakyFakeWebSocket as unknown as typeof WebSocket;
99+
100+
// Re-import to pick up the patched WebSocket binding.
101+
vi.resetModules();
102+
const { connectGateway } = await import("../src/gateway.js");
103+
const ctx = makeCtx();
104+
// Heartbeat ack timeout writes a reconnection metric — stub it.
105+
(ctx as unknown as { metrics: { write: () => Promise<void> } }).metrics = {
106+
write: vi.fn().mockResolvedValue(undefined),
107+
};
108+
(ctx.http.fetch as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
109+
ok: true,
110+
json: async () => ({ url: "wss://gateway.discord.test" }),
111+
});
112+
113+
const result = await connectGateway(ctx, "fake-token", vi.fn(), undefined, {
114+
listenForMessages: false,
115+
includeMessageContent: false,
116+
});
117+
118+
const socket = FlakyFakeWebSocket.instances[0];
119+
expect(socket).toBeDefined();
120+
121+
// HELLO with a LARGE interval — keeps the ACK timeout far in the future
122+
// (at intervalMs * 2 = 20_000ms) so the regression tick at t=10_000ms
123+
// lands before any teardown clears the heartbeat interval.
124+
const intervalMs = 10_000;
125+
socket.onmessage?.({
126+
data: JSON.stringify({
127+
op: 10,
128+
d: { heartbeat_interval: intervalMs },
129+
s: null,
130+
t: null,
131+
}),
132+
});
133+
134+
// jitter is 0, so the first heartbeat sends as soon as the next tick runs.
135+
// Advance 1ms to flush the jitter setTimeout(0) — first send fires here.
136+
vi.advanceTimersByTime(1);
137+
const firstSendCount = socket.sent.filter((p) => {
138+
try {
139+
return JSON.parse(p).op === 1;
140+
} catch {
141+
return false;
142+
}
143+
}).length;
144+
expect(firstSendCount).toBe(1);
145+
146+
// Simulate the socket transitioning to CLOSED *before* the next interval
147+
// tick — exactly the race the real bug exercised.
148+
socket.readyState = FlakyFakeWebSocket.CLOSED;
149+
150+
// Advance to t=intervalMs, which fires the FIRST interval tick. The ACK
151+
// timeout from t=0 was scheduled for t=2*intervalMs=20_000, so it has
152+
// NOT fired yet — the heartbeat interval is still live and tries to tick
153+
// on the CLOSED socket. Without the readyState guard, ws.send throws
154+
// InvalidStateError out of the setInterval callback and vitest re-throws
155+
// from advanceTimersByTime. With the guard, safeSend returns false and
156+
// no exception escapes.
157+
expect(() => vi.advanceTimersByTime(intervalMs)).not.toThrow();
158+
159+
// No additional send happened (safeSend correctly skipped the CLOSED ws).
160+
const finalSendCount = socket.sent.filter((p) => {
161+
try {
162+
return JSON.parse(p).op === 1;
163+
} catch {
164+
return false;
165+
}
166+
}).length;
167+
expect(finalSendCount).toBe(firstSendCount);
168+
169+
result.close();
170+
randomSpy.mockRestore();
171+
vi.useRealTimers();
172+
});
173+
49174
it("uses guild-only intents when message subscriptions are disabled", async () => {
50175
class FakeWebSocket {
51176
static instances: FakeWebSocket[] = [];

0 commit comments

Comments
 (0)