diff --git a/channels/discord/index.ts b/channels/discord/index.ts index 868fcca..7947ac4 100644 --- a/channels/discord/index.ts +++ b/channels/discord/index.ts @@ -20,6 +20,7 @@ * 4014; the bridge reports the exact fix instead of hot-looping. */ import { existsSync, readFileSync } from "node:fs"; +import { LOCAL_CLOSE, describeClose } from "../../lib/discord-gateway.ts"; // Managed services start from a bare environment; Zo secrets live in // /root/.zo_secrets (sourced by interactive shells). Load it when the bot token @@ -295,6 +296,8 @@ export function run(): void { let sessionId: string | null = null; let sequence: number | null = null; let gatewayUrl = GATEWAY_URL; + /** True between a RESUME and its outcome, so a rejected resume is never silent. */ + let resumePending = false; let heartbeat: ReturnType | null = null; let acked = true; let attempts = 0; @@ -329,7 +332,7 @@ export function run(): void { heartbeat = setInterval(() => { if (!acked) { stopHeartbeat(); - socket.close(4000, "heartbeat not acknowledged"); + socket.close(LOCAL_CLOSE.heartbeatUnacknowledged, "local: heartbeat not acknowledged"); return; } acked = false; @@ -341,7 +344,14 @@ export function run(): void { const socket = new Socket(gatewayUrl); socket.onopen = () => { - log(`gateway socket open (${gatewayUrl === GATEWAY_URL ? "fresh" : "resume"})`); + // Label the decision, not the URL. After the first READY the resume URL is in + // use even when the socket is about to identify fresh, so the old label read + // "(resume)" for connections that were never resuming (wazootech/data#15). + log( + sessionId === null + ? "gateway socket open (fresh identify)" + : `gateway socket open (resume from seq ${String(sequence)})`, + ); }; socket.onmessage = (event) => { @@ -356,8 +366,13 @@ export function run(): void { if (frame.op === OP.hello) { const payload = frame.d as { heartbeat_interval?: number } | null; const interval = payload?.heartbeat_interval ?? 41_250; - if (sessionId === null) identify(socket); - else send(socket, OP.resume, { token: BOT_TOKEN, session_id: sessionId, seq: sequence }); + if (sessionId === null) { + identify(socket); + } else { + resumePending = true; + log(`resuming session ${sessionId} at seq ${String(sequence)}`); + send(socket, OP.resume, { token: BOT_TOKEN, session_id: sessionId, seq: sequence }); + } startHeartbeat(socket, interval); return; } @@ -371,24 +386,34 @@ export function run(): void { } if (frame.op === OP.reconnect) { stopHeartbeat(); - socket.close(4001, "gateway asked for a reconnect"); + socket.close(LOCAL_CLOSE.reconnectRequested, "local: reconnect requested"); return; } if (frame.op === OP.invalidSession) { const resumable = frame.d === true; + log( + `session invalidated by Discord (op 9, resumable=${String(resumable)})${resumePending ? ", in reply to our resume" : ""}`, + ); + resumePending = false; if (!resumable) { sessionId = null; sequence = null; gatewayUrl = GATEWAY_URL; } stopHeartbeat(); - socket.close(4002, "invalid session"); + socket.close(LOCAL_CLOSE.invalidSession, "local: invalid session"); return; } if (frame.op !== OP.dispatch) return; if (frame.t === "READY") { const payload = frame.d as ReadyPayload | null; + if (resumePending) { + log( + "the resume was rejected: Discord answered READY instead of RESUMED, so the session was dropped and a fresh one is identifying", + ); + } + resumePending = false; sessionId = payload?.session_id ?? null; if (typeof payload?.resume_gateway_url === "string") gatewayUrl = payload.resume_gateway_url; BOT_USER_ID = payload?.user?.id ?? BOT_USER_ID; @@ -405,6 +430,7 @@ export function run(): void { } if (frame.t === "RESUMED") { attempts = 0; + resumePending = false; log("session resumed"); return; } @@ -424,31 +450,32 @@ export function run(): void { socket.onclose = (event) => { stopHeartbeat(); + resumePending = false; const code = event.code; - const reason = event.reason.length > 0 ? ` (${event.reason})` : ""; + const described = describeClose(code, event.reason); if (code === 4014) { log( "gateway refused the connection with 4014 (disallowed intents): enable Message Content Intent for the Data application (Discord Developer Portal -> Data -> Bot -> Privileged Gateway Intents), then this process connects on its next attempt.", ); - reconnect(DISALLOWED_INTENTS_DELAY_MS, `4014${reason}`); + reconnect(DISALLOWED_INTENTS_DELAY_MS, described); return; } if (code === 4004) { log("gateway refused the connection with 4004 (authentication failed): DATA_DISCORD_BOT_TOKEN is wrong or rotated"); - reconnect(MAX_BACKOFF_MS, `4004${reason}`); + reconnect(MAX_BACKOFF_MS, described); return; } if (code === 4013) { log("gateway refused the connection with 4013 (invalid intents): the requested intent bits are not valid for this application"); - reconnect(MAX_BACKOFF_MS, `4013${reason}`); + reconnect(MAX_BACKOFF_MS, described); return; } if (code === 4010 || code === 4011) { - log(`gateway closed with ${String(code)}${reason}: sharding must not be changed while resuming`); + log(`${described}: sharding must not be changed while resuming`); sessionId = null; sequence = null; gatewayUrl = GATEWAY_URL; - reconnect(MIN_BACKOFF_MS, `close ${String(code)}`); + reconnect(MIN_BACKOFF_MS, described); return; } if (code === 4007 || code === 4008 || code === 4009) { @@ -459,7 +486,7 @@ export function run(): void { attempts += 1; const backoff = Math.min(MAX_BACKOFF_MS, MIN_BACKOFF_MS * 2 ** (attempts - 1)); const jittered = backoff / 2 + Math.random() * (backoff / 2); - reconnect(jittered, `close ${String(code)}${reason}`); + reconnect(jittered, described); }; } diff --git a/lib/discord-gateway.test.ts b/lib/discord-gateway.test.ts new file mode 100644 index 0000000..c78a11f --- /dev/null +++ b/lib/discord-gateway.test.ts @@ -0,0 +1,37 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { LOCAL_CLOSE, describeClose, isLocalClose } from "./discord-gateway.ts"; + +test("names a Discord close as Discord's, never as the local one", () => { + // 4002 is Discord's decode error. It used to be indistinguishable in the log + // from this bridge's own invalid-session close, which is what #15 hit. + assert.equal(describeClose(4002, ""), "Discord close 4002 (decode error)"); + assert.equal(describeClose(4002, "bad frame"), "Discord close 4002 (decode error) (bad frame)"); + assert.equal(describeClose(4014, ""), "Discord close 4014 (disallowed intent(s))"); + assert.equal(describeClose(4004, ""), "Discord close 4004 (authentication failed)"); + assert.equal(describeClose(4007, ""), "Discord close 4007 (invalid seq)"); +}); + +test("names a local close as local, and never collides with Discord's range", () => { + assert.equal(describeClose(LOCAL_CLOSE.invalidSession, "local: invalid session"), "local close 4902 (local: invalid session)"); + assert.equal(isLocalClose(LOCAL_CLOSE.invalidSession), true); + assert.equal(isLocalClose(LOCAL_CLOSE.heartbeatUnacknowledged), true); + assert.equal(isLocalClose(LOCAL_CLOSE.reconnectRequested), true); + for (const code of Object.values(LOCAL_CLOSE)) { + assert.ok(code >= 4900, `local close ${String(code)} must not collide with Discord's 4xxx codes`); + } +}); + +test("a Discord code is never mistaken for a local one", () => { + for (const code of [4000, 4001, 4002, 4003, 4004, 4005, 4007, 4008, 4009, 4010, 4011, 4012, 4013, 4014]) { + assert.equal(isLocalClose(code), false, `${String(code)} is Discord's, not ours`); + } +}); + +test("describes transport closes and anything unrecognized", () => { + assert.equal(describeClose(1006, ""), "transport close 1006 (no close frame: the connection ended)"); + assert.equal(describeClose(1000, ""), "transport close 1000 (normal)"); + assert.equal(describeClose(1005, ""), "close 1005"); + assert.equal(describeClose(1005, "nope"), "close 1005 (nope)"); +}); diff --git a/lib/discord-gateway.ts b/lib/discord-gateway.ts new file mode 100644 index 0000000..ee91dee --- /dev/null +++ b/lib/discord-gateway.ts @@ -0,0 +1,62 @@ +/** + * Close-code vocabulary for Data's hand-rolled Discord gateway. + * + * Discord's documented gateway close codes all sit in the 4000-4999 range, and + * `4002` is Discord's *decode error*. The bridge's own local closes used the same + * range, so a log line reading `close 4002` could be either Discord rejecting + * something we sent or our own reaction to an invalid session, and neither the + * code nor the reason told them apart. wazootech/data#15 was diagnosed under that + * ambiguity. Local closes now live in a private-use range and name themselves. + */ + +/** Close codes this process raises itself. Discord never sends these. */ +export const LOCAL_CLOSE = { + heartbeatUnacknowledged: 4900, + reconnectRequested: 4901, + invalidSession: 4902, +} as const; + +const LOCAL_CLOSE_NAMES: Record = { + 4900: "heartbeat not acknowledged", + 4901: "local: the gateway asked for a reconnect", + 4902: "local: invalid session", +}; + +const DISCORD_CLOSE_NAMES: Record = { + 4000: "unknown error", + 4001: "unknown opcode", + 4002: "decode error", + 4003: "not authenticated", + 4004: "authentication failed", + 4005: "already authenticated", + 4007: "invalid seq", + 4008: "rate limited", + 4009: "session timed out", + 4010: "invalid shard", + 4011: "sharding required", + 4012: "invalid API version", + 4013: "invalid intent(s)", + 4014: "disallowed intent(s)", +}; + +/** + * One unambiguous phrase for a close event, naming which side raised it. + * + * `4002` is Discord's decode error, so it must never read like the local + * invalid-session close: everything Discord sends is labelled `Discord close`. + */ +export function describeClose(code: number, reason: string): string { + const local = LOCAL_CLOSE_NAMES[code]; + if (local !== undefined) return `local close ${String(code)} (${local})`; + const detail = reason.length > 0 ? ` (${reason})` : ""; + const discord = DISCORD_CLOSE_NAMES[code]; + if (discord !== undefined) return `Discord close ${String(code)} (${discord})${detail}`; + if (code === 1000) return `transport close 1000 (normal)${detail}`; + if (code === 1006) return "transport close 1006 (no close frame: the connection ended)"; + return `close ${String(code)}${detail}`; +} + +/** True when this process raised the close itself. */ +export function isLocalClose(code: number): boolean { + return LOCAL_CLOSE_NAMES[code] !== undefined; +}