test(bot): cover the back→re-pair close-leak regression
Extract the pair-handler's close-event decision into a pure helper decidePairListenerOnClose(warmingUp, restartRequired) returning one of ignore-leaked-close / post-pair-restart / treat-as-timeout. Refactor pair-handler to call the helper instead of the inline if-chain. New tests in pair-state.test.ts: - warmingUp=true → ignore-leaked-close (regression: prior session's close racing the new listener) - warmingUp=true + restartRequired=true → still ignore (defense in depth — a stale 515 must not hand control to the reconnect path) - warmingUp=false + restartRequired=true → post-pair-restart - warmingUp=false → treat-as-timeout Bot suite goes from 60 → 64 tests, all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
68668ef2cd
commit
7af7aa35d0
@ -10,6 +10,7 @@ import { renderQrPng } from "../whatsapp/qr-renderer.js";
|
|||||||
import { syncGroupsForAccount } from "../whatsapp/group-sync.js";
|
import { syncGroupsForAccount } from "../whatsapp/group-sync.js";
|
||||||
import { writeAuditLog } from "../audit.js";
|
import { writeAuditLog } from "../audit.js";
|
||||||
import { pgNotifyWeb } from "./notify.js";
|
import { pgNotifyWeb } from "./notify.js";
|
||||||
|
import { decidePairListenerOnClose } from "./pair-state.js";
|
||||||
|
|
||||||
const PAIR_TIMEOUT_MS = 5 * 60 * 1000;
|
const PAIR_TIMEOUT_MS = 5 * 60 * 1000;
|
||||||
const offByAccount = new Map<string, () => void>();
|
const offByAccount = new Map<string, () => void>();
|
||||||
@ -150,40 +151,33 @@ export async function handleStartPairing(accountId: string): Promise<void> {
|
|||||||
count: synced,
|
count: synced,
|
||||||
});
|
});
|
||||||
off();
|
off();
|
||||||
} else if (event.type === "close" && event.restartRequired) {
|
|
||||||
// After the user scans, WhatsApp tells Baileys to "restart"
|
|
||||||
// the connection. The socket closes with status 515 and the
|
|
||||||
// session-manager will reopen it with the new credentials —
|
|
||||||
// the next `open` event is what completes the pairing.
|
|
||||||
// This is NOT a failure: keep the listener attached so we see
|
|
||||||
// that subsequent `open` event, and don't surface a timeout
|
|
||||||
// to the UI. The DB row stays in `pending` until `open`.
|
|
||||||
logger.info(
|
|
||||||
{ accountId: id },
|
|
||||||
"pair: restart-required close (post-pair reconnect) — keeping listener alive",
|
|
||||||
);
|
|
||||||
// The session-manager handles the actual reconnect; nothing to
|
|
||||||
// do here other than NOT tear our listener / DB state down.
|
|
||||||
} else if (event.type === "close") {
|
} else if (event.type === "close") {
|
||||||
// Swallow the leaked close from the *previous* pairing attempt
|
const decision = decidePairListenerOnClose({
|
||||||
// being torn down inside this handler's own
|
warmingUp: pairingWarmingUp.has(id),
|
||||||
// `await sessionManager.stop()`. The old session's close is
|
restartRequired: event.restartRequired,
|
||||||
// broadcast asynchronously and lands here AFTER we've attached
|
});
|
||||||
// the new listener; treating it as a real timeout would flash
|
if (decision === "ignore-leaked-close") {
|
||||||
// "Pairing timed out" before any QR even arrives.
|
|
||||||
if (pairingWarmingUp.has(id)) {
|
|
||||||
logger.info(
|
logger.info(
|
||||||
{ accountId: id },
|
{ accountId: id },
|
||||||
"pair: ignoring close from previous attempt while warming up",
|
"pair: ignoring close from previous attempt while warming up",
|
||||||
);
|
);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// During the pairing window, any other close means the QR window
|
if (decision === "post-pair-restart") {
|
||||||
// ended without a successful link — Baileys' default is to
|
// After the user scans, WhatsApp tells Baileys to "restart"
|
||||||
// close after exhausting QR refs (~2.5 min). Surface this to
|
// the connection. The socket closes with status 515 and the
|
||||||
// the UI so the user gets a "pairing timed out" screen, and
|
// session-manager will reopen it with the new credentials —
|
||||||
// park the row in a stable state so it shows up cleanly on
|
// the next `open` event finishes the pairing. Keep the
|
||||||
// the accounts list with a "Re-pair" affordance.
|
// listener attached and don't surface a timeout to the UI.
|
||||||
|
logger.info(
|
||||||
|
{ accountId: id },
|
||||||
|
"pair: restart-required close (post-pair reconnect) — keeping listener alive",
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// decision === "treat-as-timeout": ephemeral close on a live
|
||||||
|
// attempt. Park the row as `unpaired` and push session.timeout
|
||||||
|
// so the operator sees the "Re-pair" affordance.
|
||||||
const t = pairTimeouts.get(id);
|
const t = pairTimeouts.get(id);
|
||||||
if (t) {
|
if (t) {
|
||||||
clearTimeout(t);
|
clearTimeout(t);
|
||||||
|
|||||||
@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
|
|||||||
import {
|
import {
|
||||||
decideOnPairClose,
|
decideOnPairClose,
|
||||||
decideOnPairTimeout,
|
decideOnPairTimeout,
|
||||||
|
decidePairListenerOnClose,
|
||||||
shouldAutoReconnect,
|
shouldAutoReconnect,
|
||||||
} from "./pair-state.js";
|
} from "./pair-state.js";
|
||||||
|
|
||||||
@ -82,3 +83,45 @@ describe("shouldAutoReconnect", () => {
|
|||||||
expect(shouldAutoReconnect({ loggedOut: false, hasEverConnected: false })).toBe(false);
|
expect(shouldAutoReconnect({ loggedOut: false, hasEverConnected: false })).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("decidePairListenerOnClose (back→re-pair flicker regression)", () => {
|
||||||
|
it("ignores a close while warming up — even if also restartRequired", () => {
|
||||||
|
// The exact bug: stop() was awaited, listener attached, then the OLD
|
||||||
|
// session's close arrives and races our new listener. Warming-up
|
||||||
|
// wins over every other branch so the UI never sees a spurious
|
||||||
|
// session.timeout before the new QR is rendered.
|
||||||
|
expect(
|
||||||
|
decidePairListenerOnClose({ warmingUp: true, restartRequired: false }),
|
||||||
|
).toBe("ignore-leaked-close");
|
||||||
|
expect(
|
||||||
|
decidePairListenerOnClose({ warmingUp: true, restartRequired: true }),
|
||||||
|
).toBe("ignore-leaked-close");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("treats a close on a live attempt (warmingUp=false) as a real timeout", () => {
|
||||||
|
// Refs exhausted, network blip, etc. — operator gets the
|
||||||
|
// "Pairing timed out" screen and a Re-pair affordance.
|
||||||
|
expect(
|
||||||
|
decidePairListenerOnClose({ warmingUp: false, restartRequired: false }),
|
||||||
|
).toBe("treat-as-timeout");
|
||||||
|
expect(decidePairListenerOnClose({ warmingUp: false })).toBe("treat-as-timeout");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves the restart-required (post-pair-success) branch when not warming up", () => {
|
||||||
|
// Status 515 close: the session-manager will reconnect and the next
|
||||||
|
// `open` finishes the pair. We must NOT push session.timeout here.
|
||||||
|
expect(
|
||||||
|
decidePairListenerOnClose({ warmingUp: false, restartRequired: true }),
|
||||||
|
).toBe("post-pair-restart");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("warming-up overrides restartRequired so 515 from a stale session is also swallowed", () => {
|
||||||
|
// Defense-in-depth: if Baileys' restart-required close from the OLD
|
||||||
|
// session somehow leaks through, treating it as a real 515 would
|
||||||
|
// KEEP the listener attached forever (no reconnect comes from a
|
||||||
|
// session we just stopped). Ignore it entirely until a fresh qr/open.
|
||||||
|
expect(
|
||||||
|
decidePairListenerOnClose({ warmingUp: true, restartRequired: true }),
|
||||||
|
).toBe("ignore-leaked-close");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@ -80,3 +80,37 @@ export function decideOnPairTimeout({ current }: { current: AccountStatus }): St
|
|||||||
if (current !== "pending") return null;
|
if (current !== "pending") return null;
|
||||||
return { next: "unpaired", clearQrPng: true };
|
return { next: "unpaired", clearQrPng: true };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide how the pair-handler should react to a `close` event delivered
|
||||||
|
* to its listener. Three outcomes:
|
||||||
|
*
|
||||||
|
* - "ignore-leaked-close": the new attempt is still warming up and
|
||||||
|
* we're seeing the OLD session's tail close. Do nothing — don't
|
||||||
|
* emit timeout to the UI, don't touch the DB row.
|
||||||
|
* - "post-pair-restart": status-515 close from a successful scan.
|
||||||
|
* The session-manager will reconnect; we keep the listener alive
|
||||||
|
* and wait for the subsequent `open` event.
|
||||||
|
* - "treat-as-timeout": a real ephemeral close on a live attempt
|
||||||
|
* (refs exhausted, etc.). Park the row as `unpaired` and push
|
||||||
|
* `session.timeout` to the UI.
|
||||||
|
*
|
||||||
|
* Captures the regression where, after the user pulled up a QR and
|
||||||
|
* navigated back, clicking Pair again would instantly flash "Pairing
|
||||||
|
* timed out" because the await on stop() returned before
|
||||||
|
* sessionManager.handleEvent finished broadcasting the old session's
|
||||||
|
* close — and the new listener was already attached.
|
||||||
|
*/
|
||||||
|
export type PairListenerCloseDecision =
|
||||||
|
| "ignore-leaked-close"
|
||||||
|
| "post-pair-restart"
|
||||||
|
| "treat-as-timeout";
|
||||||
|
|
||||||
|
export function decidePairListenerOnClose(input: {
|
||||||
|
warmingUp: boolean;
|
||||||
|
restartRequired?: boolean;
|
||||||
|
}): PairListenerCloseDecision {
|
||||||
|
if (input.warmingUp) return "ignore-leaked-close";
|
||||||
|
if (input.restartRequired) return "post-pair-restart";
|
||||||
|
return "treat-as-timeout";
|
||||||
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user