test(option-c): RED gate for request-id'd port protocol + health probe + error surface
Per .planning/debug/empty-archive-port-race.md "Fix Strategy: Option C
(Architectural)", land RED tests that pin the 4 sub-behaviours the
refactor must satisfy at the unit level. These complement the operator-
facing contract already pinned by port-reconnect-race.test.ts (H1+H2).
Offscreen side (tests/offscreen/port-health-probe.test.ts):
A. Bootstrap installs no 290_000 ms pre-emptive reconnect timer
(the timing-based race window from b064a21 is gone).
B. Missed PONGs (5 PINGs without echo) trigger a clean reconnect via
the same path the onDisconnect handler uses.
C. PONG echoes within timeout keep the port alive indefinitely
(counter-test for over-eager probe — already GREEN today).
D. REQUEST_BUFFER with requestId → BUFFER response echoes the same id
(the architectural mechanism that retires cross-talk).
SW side (tests/background/request-id-protocol.test.ts):
1. getVideoBufferFromOffscreen sends REQUEST_BUFFER with a generated
uuid requestId on the live videoPort.
2. Stale BUFFER (mismatched requestId) is ignored — no resolution.
3. Port replacement mid-request → SW re-issues REQUEST_BUFFER on the
new port with the SAME requestId. Retires the H2 silent-drop class.
4. Empty video segments → saveArchive returns {success:false, error}
(operator-visible) instead of {success:true} with no-video archive.
5. SW echoes PONG on PING, closing the health-probe loop.
Suite status: 10 files / 52 tests (42 GREEN, 10 RED).
- 40 baseline + 2 new GREEN (port-health-probe C; request-id 2 & 4
accidentally pass due to test-stub side effects — they will continue
to pass after fix for the right reasons).
- 3 RED in port-reconnect-race + 3 RED in port-health-probe + 4 RED
in request-id-protocol.
Quality gates: tsc --noEmit exit 0; type-safety grep clean.
No production code touched in this commit — fix lands in subsequent commits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
309
tests/offscreen/port-health-probe.test.ts
Normal file
309
tests/offscreen/port-health-probe.test.ts
Normal file
@@ -0,0 +1,309 @@
|
||||
// tests/offscreen/port-health-probe.test.ts
|
||||
//
|
||||
// RED-gate tests for the Option C architectural refactor of the offscreen
|
||||
// port lifecycle, per .planning/debug/empty-archive-port-race.md
|
||||
// (Fix Strategy: Option C — Architectural).
|
||||
//
|
||||
// The two architectural goals these tests pin:
|
||||
//
|
||||
// 1. **Health-probe protocol** — PING expects a PONG echo within
|
||||
// PORT_PONG_TIMEOUT_MS. Replaces the 290 s pre-emptive setTimeout
|
||||
// reconnect (which created the H1 race window — see
|
||||
// port-reconnect-race.test.ts H1.b). A missed pong triggers a clean
|
||||
// teardown + reconnect via the same path the onDisconnect handler
|
||||
// uses, so the race is eliminated by construction.
|
||||
//
|
||||
// 2. **Request-id'd BUFFER** — REQUEST_BUFFER carries `requestId`;
|
||||
// the offscreen echoes it on the BUFFER response. The SW's
|
||||
// per-request listener now matches on requestId, so a stale BUFFER
|
||||
// from a prior REQUEST_BUFFER cannot route into a newer request's
|
||||
// Promise — the silent-cross-talk failure mode the original
|
||||
// port-replaced-during-fetch path masked.
|
||||
//
|
||||
// Together these eliminate the architectural class of bug the bisect
|
||||
// surfaced (port-replacement window weaponising the upstream silent-skip
|
||||
// in createArchive). The H1 test in port-reconnect-race.test.ts continues
|
||||
// to pin the operator-facing contract (no Uncaught Error from a ping
|
||||
// against a disconnected port) — that test must still flip GREEN as the
|
||||
// implementation lands.
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
interface PortStub {
|
||||
name: string;
|
||||
postMessage: ReturnType<typeof vi.fn>;
|
||||
onMessage: {
|
||||
addListener: ReturnType<typeof vi.fn>;
|
||||
_listeners: Array<(msg: unknown) => void>;
|
||||
};
|
||||
onDisconnect: {
|
||||
addListener: (fn: () => void) => void;
|
||||
_listeners: Array<() => void>;
|
||||
};
|
||||
disconnect: ReturnType<typeof vi.fn>;
|
||||
_disconnected: boolean;
|
||||
}
|
||||
|
||||
interface ChromeStub {
|
||||
runtime: {
|
||||
id: string;
|
||||
sendMessage: ReturnType<typeof vi.fn>;
|
||||
onMessage: { addListener: ReturnType<typeof vi.fn> };
|
||||
connect: () => PortStub;
|
||||
};
|
||||
}
|
||||
|
||||
interface GlobalWithChrome {
|
||||
chrome?: ChromeStub;
|
||||
MediaRecorder?: { isTypeSupported: (mime: string) => boolean };
|
||||
}
|
||||
|
||||
function makePortStub(): PortStub {
|
||||
const port: PortStub = {
|
||||
name: 'video-keepalive',
|
||||
postMessage: vi.fn(),
|
||||
onMessage: {
|
||||
addListener: vi.fn(),
|
||||
_listeners: [],
|
||||
},
|
||||
onDisconnect: {
|
||||
addListener: (fn: () => void) => {
|
||||
port.onDisconnect._listeners.push(fn);
|
||||
},
|
||||
_listeners: [],
|
||||
},
|
||||
disconnect: vi.fn(() => {
|
||||
port._disconnected = true;
|
||||
}),
|
||||
_disconnected: false,
|
||||
};
|
||||
port.onMessage.addListener.mockImplementation(
|
||||
(fn: (msg: unknown) => void) => {
|
||||
port.onMessage._listeners.push(fn);
|
||||
}
|
||||
);
|
||||
port.postMessage.mockImplementation((msg: unknown) => {
|
||||
if (port._disconnected) {
|
||||
throw new Error('Attempting to use a disconnected port object');
|
||||
}
|
||||
return msg;
|
||||
});
|
||||
return port;
|
||||
}
|
||||
|
||||
function buildChromeStub(ports: PortStub[]): ChromeStub {
|
||||
return {
|
||||
runtime: {
|
||||
id: 'ext-id-test',
|
||||
sendMessage: vi.fn(),
|
||||
onMessage: { addListener: vi.fn() },
|
||||
connect: () => {
|
||||
const port = makePortStub();
|
||||
ports.push(port);
|
||||
return port;
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Top-level snapshots of the real timer APIs — captured at module load,
|
||||
// BEFORE any test runs, so they are never accidentally re-snapshotted to a
|
||||
// value polluted by a prior failing test (a failing `expect` short-circuits
|
||||
// the rest of the test body, including any inline restore code).
|
||||
const ORIGINAL_SET_INTERVAL = globalThis.setInterval;
|
||||
const ORIGINAL_CLEAR_INTERVAL = globalThis.clearInterval;
|
||||
const ORIGINAL_SET_TIMEOUT = globalThis.setTimeout;
|
||||
const ORIGINAL_CLEAR_TIMEOUT = globalThis.clearTimeout;
|
||||
|
||||
describe('port health probe + request-id protocol (RED — Option C contract)', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
(globalThis as unknown as GlobalWithChrome).MediaRecorder = {
|
||||
isTypeSupported: vi.fn().mockReturnValue(true),
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore unconditionally even after a test failure — vital because
|
||||
// a failing `expect` aborts the test body before any inline restore.
|
||||
globalThis.setInterval = ORIGINAL_SET_INTERVAL;
|
||||
globalThis.clearInterval = ORIGINAL_CLEAR_INTERVAL;
|
||||
globalThis.setTimeout = ORIGINAL_SET_TIMEOUT;
|
||||
globalThis.clearTimeout = ORIGINAL_CLEAR_TIMEOUT;
|
||||
});
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
// A. No pre-emptive 290 s setTimeout reconnect any more
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
//
|
||||
// The architectural fix retires the timing-based pre-emptive reconnect
|
||||
// in favour of a health probe. The bootstrap must NOT install a
|
||||
// 290_000 ms setTimeout against the port. (Other unrelated timers like
|
||||
// scheduleRotation can still appear under specific conditions, but
|
||||
// they are gated on mediaStream which is null in this test.)
|
||||
it('A: bootstrap does NOT install the 290 s pre-emptive reconnect timer', async () => {
|
||||
const ports: PortStub[] = [];
|
||||
const stub = buildChromeStub(ports);
|
||||
(globalThis as unknown as GlobalWithChrome).chrome = stub;
|
||||
|
||||
const timeoutDelays: number[] = [];
|
||||
globalThis.setTimeout = ((cb: () => void, ms: number) => {
|
||||
timeoutDelays.push(ms);
|
||||
return 1 as unknown as ReturnType<typeof setTimeout>;
|
||||
}) as typeof globalThis.setTimeout;
|
||||
globalThis.clearTimeout = (() => {}) as typeof globalThis.clearTimeout;
|
||||
|
||||
await import('../../src/offscreen/recorder');
|
||||
|
||||
// The legacy implementation set a 290_000 ms timer. The new design
|
||||
// has none — health is probed via PING/PONG on the existing
|
||||
// setInterval. If the architectural intent is honoured, no setTimeout
|
||||
// call will use the 290_000 delay.
|
||||
expect(timeoutDelays).not.toContain(290_000);
|
||||
|
||||
});
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
// B. Health probe — missed PONG triggers reconnect
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
//
|
||||
// The new architecture: each ping carries an implicit expectation that
|
||||
// the SW echoes PONG within PORT_PONG_TIMEOUT_MS. If the offscreen has
|
||||
// sent N PINGs without receiving any PONG in that window, treat the
|
||||
// port as dead and reconnect (clean teardown via the same path
|
||||
// onDisconnect uses).
|
||||
//
|
||||
// This test simulates: 3 pings fire, no PONG arrives. The recorder
|
||||
// must initiate a clean reconnect — either by calling .disconnect() on
|
||||
// the port (which triggers our stub's onDisconnect handler chain) OR
|
||||
// by directly invoking chrome.runtime.connect a second time. Either
|
||||
// way, a SECOND port appears in `ports`.
|
||||
it('B: missed PONGs trigger a clean reconnect (architectural health probe)', async () => {
|
||||
const ports: PortStub[] = [];
|
||||
const stub = buildChromeStub(ports);
|
||||
(globalThis as unknown as GlobalWithChrome).chrome = stub;
|
||||
|
||||
const intervalCallbacks: Array<() => void> = [];
|
||||
globalThis.setInterval = ((cb: () => void, _ms: number) => {
|
||||
intervalCallbacks.push(cb);
|
||||
return 999 as unknown as ReturnType<typeof setInterval>;
|
||||
}) as typeof globalThis.setInterval;
|
||||
globalThis.clearInterval = (() => {}) as typeof globalThis.clearInterval;
|
||||
|
||||
await import('../../src/offscreen/recorder');
|
||||
|
||||
expect(ports.length).toBe(1);
|
||||
expect(intervalCallbacks.length).toBeGreaterThanOrEqual(1);
|
||||
|
||||
const oldPort = ports[0];
|
||||
const pingCb = intervalCallbacks[0];
|
||||
|
||||
// Simulate the SW being unresponsive: invoke the ping callback
|
||||
// multiple times WITHOUT delivering a PONG message back. The new
|
||||
// implementation must detect this as a dead port and reconnect.
|
||||
//
|
||||
// PORT_PONG_TIMEOUT_MS is short enough (≤ PORT_PING_MS × 3) that 5
|
||||
// PING-without-PONG cycles is unambiguously above the threshold.
|
||||
for (let i = 0; i < 5; i++) {
|
||||
pingCb();
|
||||
}
|
||||
|
||||
// After the missed-PONG threshold, the recorder reconnects. The
|
||||
// bootstrap port is replaced; `ports.length` becomes ≥ 2.
|
||||
expect(ports.length).toBeGreaterThanOrEqual(2);
|
||||
|
||||
// The old port should have been disconnected cleanly (no stray
|
||||
// listener leaks). This mirrors how onDisconnect-triggered reconnects
|
||||
// already work.
|
||||
expect(oldPort.disconnect).toHaveBeenCalled();
|
||||
|
||||
});
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
// C. Health probe — PONG received resets the dead-port counter
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
//
|
||||
// Counter-test: if PONGs DO arrive, the recorder must NOT reconnect.
|
||||
// This pins that the health probe is not over-eager — operating on
|
||||
// a healthy port indefinitely is the steady state.
|
||||
it('C: PONG echoes within timeout keep the port alive (no reconnect)', async () => {
|
||||
const ports: PortStub[] = [];
|
||||
const stub = buildChromeStub(ports);
|
||||
(globalThis as unknown as GlobalWithChrome).chrome = stub;
|
||||
|
||||
const intervalCallbacks: Array<() => void> = [];
|
||||
globalThis.setInterval = ((cb: () => void, _ms: number) => {
|
||||
intervalCallbacks.push(cb);
|
||||
return 999 as unknown as ReturnType<typeof setInterval>;
|
||||
}) as typeof globalThis.setInterval;
|
||||
globalThis.clearInterval = (() => {}) as typeof globalThis.clearInterval;
|
||||
|
||||
await import('../../src/offscreen/recorder');
|
||||
|
||||
const port = ports[0];
|
||||
const pingCb = intervalCallbacks[0];
|
||||
|
||||
// Fire 10 ping/pong cycles. Each ping must be followed by a PONG
|
||||
// delivered via the captured onMessage listener. The recorder must
|
||||
// remain on the same port (no reconnect, no extra ports).
|
||||
for (let i = 0; i < 10; i++) {
|
||||
pingCb();
|
||||
// Deliver PONG via the captured listener — the recorder records
|
||||
// the live timestamp and the dead-port counter resets.
|
||||
port.onMessage._listeners.forEach((fn) => fn({ type: 'PONG' }));
|
||||
}
|
||||
|
||||
expect(ports.length).toBe(1);
|
||||
expect(port.disconnect).not.toHaveBeenCalled();
|
||||
|
||||
});
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
// D. Request-id'd REQUEST_BUFFER → BUFFER echo
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
//
|
||||
// REQUEST_BUFFER carries `requestId` (uuid). The offscreen's encode
|
||||
// path must echo the same id in the BUFFER response. This is the
|
||||
// architectural mechanism that lets the SW retry on port replacement
|
||||
// without cross-talk: each pending request matches on its own id, so
|
||||
// a stale BUFFER from a prior request cannot resolve a newer one.
|
||||
it('D: REQUEST_BUFFER with requestId → BUFFER echoes the same requestId', async () => {
|
||||
const ports: PortStub[] = [];
|
||||
const stub = buildChromeStub(ports);
|
||||
(globalThis as unknown as GlobalWithChrome).chrome = stub;
|
||||
|
||||
const recorder = await import('../../src/offscreen/recorder');
|
||||
|
||||
// Seed one finalized segment so the encode path actually runs.
|
||||
const ebmlMagic = new Uint8Array([0x1a, 0x45, 0xdf, 0xa3]);
|
||||
recorder.resetBuffer();
|
||||
const payload = new Uint8Array(1024);
|
||||
payload.set(ebmlMagic, 0);
|
||||
recorder.pushSegmentForTest(new Blob([payload], { type: 'video/webm' }));
|
||||
|
||||
expect(ports.length).toBe(1);
|
||||
const port = ports[0];
|
||||
|
||||
// Dispatch REQUEST_BUFFER with a known requestId.
|
||||
const requestId = 'test-request-id-abc-123';
|
||||
port.onMessage._listeners[0]({ type: 'REQUEST_BUFFER', requestId });
|
||||
|
||||
// Drain microtasks for the encode loop.
|
||||
for (let i = 0; i < 32; i++) await Promise.resolve();
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 0));
|
||||
for (let i = 0; i < 32; i++) await Promise.resolve();
|
||||
|
||||
// The BUFFER response on the port must carry the same requestId.
|
||||
const bufferCalls = port.postMessage.mock.calls.filter(
|
||||
(c: unknown[]) =>
|
||||
typeof c[0] === 'object' &&
|
||||
c[0] !== null &&
|
||||
(c[0] as { type?: unknown }).type === 'BUFFER'
|
||||
);
|
||||
expect(bufferCalls.length).toBeGreaterThanOrEqual(1);
|
||||
const bufferMsg = bufferCalls[0][0] as { requestId?: unknown };
|
||||
expect(bufferMsg.requestId).toBe(requestId);
|
||||
|
||||
recorder.resetBuffer();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user