From fcdade8ef073a49554113501e0c2ed0925d6504a Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Sat, 3 Oct 2026 13:18:07 +0500 Subject: [PATCH] fix(client): tolerate transient control-poll failures in useVoice A single failed GET /voice/calls/:id poll ended the call immediately, even for a short network flap the peer would have recovered from. Track consecutive poll failures on the session and only declare the control connection lost after three in a row; any successful poll resets the count. Mirrors the grace period #26 gives the peer connection. Adds regression tests covering the new behavior. Fixes #38 --- package-lock.json | 120 ++++++++++++++++++------- package.json | 2 + src/client/useVoice.ts | 16 +++- tests/use-voice-control-poll.test.ts | 125 +++++++++++++++++++++++++++ 4 files changed, 230 insertions(+), 33 deletions(-) create mode 100644 tests/use-voice-control-poll.test.ts diff --git a/package-lock.json b/package-lock.json index 895d8ce..0e3a6d8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43,11 +43,13 @@ "@types/node": "^26.6.3", "@types/react": "^19.3.0", "@types/react-dom": "^19.3.0", + "@types/react-test-renderer": "^19.3.0", "@vitejs/plugin-react": "^6.1.1", "concurrently": "^10.0.5", "eslint": "^10.11.0", "eslint-plugin-react-hooks": "^7.1.1", "prettier": "^3.9.9", + "react-test-renderer": "^19.3.0", "tsx": "^4.23.15", "typescript": "^6.0.3", "typescript-eslint": "^8.71.0", @@ -2234,6 +2236,7 @@ "cpu": [ "ppc64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2250,6 +2253,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2266,6 +2270,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2282,6 +2287,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2298,6 +2304,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2314,6 +2321,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2330,6 +2338,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2346,6 +2355,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2362,6 +2372,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2378,6 +2389,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2394,6 +2406,7 @@ "cpu": [ "ia32" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2410,6 +2423,7 @@ "cpu": [ "loong64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2426,6 +2440,7 @@ "cpu": [ "mips64el" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2442,6 +2457,7 @@ "cpu": [ "ppc64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2458,6 +2474,7 @@ "cpu": [ "riscv64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2474,6 +2491,7 @@ "cpu": [ "s390x" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2490,6 +2508,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2506,6 +2525,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2522,6 +2542,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2538,6 +2559,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2554,6 +2576,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2570,6 +2593,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2586,6 +2610,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2602,6 +2627,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2618,6 +2644,7 @@ "cpu": [ "ia32" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2634,6 +2661,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4083,6 +4111,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4099,6 +4128,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4115,6 +4145,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4131,6 +4162,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4147,6 +4179,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4163,6 +4196,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4179,9 +4213,7 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4198,9 +4230,7 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4217,9 +4247,7 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4236,9 +4264,7 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4255,9 +4281,7 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4274,9 +4298,7 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4293,6 +4315,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4309,6 +4332,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -4325,6 +4349,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -5842,6 +5867,16 @@ "@types/react": "^19.3.0" } }, + "node_modules/@types/react-test-renderer": { + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/@types/react-test-renderer/-/react-test-renderer-19.3.0.tgz", + "integrity": "sha512-TX4p2D4Z2oigM3SXU7+QyLSUnSR0vR1S1Pfxm/qFL9nHQ3jclgi6ioGoWQmlIr1DeKRVmPJrmNlhgM0JlWlLUg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/react": "*" + } + }, "node_modules/@types/retry": { "version": "0.12.0", "resolved": "https://registry.npmjs.org/@types/retry/-/retry-0.12.0.tgz", @@ -8088,7 +8123,7 @@ "version": "0.28.2", "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.28.2.tgz", "integrity": "sha512-HKVLS8dvII+xoKW9kmqxbRKrnWEXfJJr/FZhhJmiqIB0e053QNYFqOBouTMO/k5sID4MvCiUCvv8b9M4h32wIA==", - "devOptional": true, + "dev": true, "hasInstallScript": true, "license": "MIT", "bin": { @@ -8762,6 +8797,7 @@ "version": "2.3.3", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz", "integrity": "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==", + "dev": true, "hasInstallScript": true, "license": "MIT", "optional": true, @@ -10029,6 +10065,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10049,6 +10086,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10069,6 +10107,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10089,6 +10128,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10109,6 +10149,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10129,9 +10170,7 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10152,9 +10191,7 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10175,9 +10212,7 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10198,9 +10233,7 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10221,6 +10254,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -10241,6 +10275,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -12736,6 +12771,27 @@ } } }, + "node_modules/react-test-renderer": { + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/react-test-renderer/-/react-test-renderer-19.3.0.tgz", + "integrity": "sha512-AUxgv+hRbqjRgx4xVwiWL6cvDDKPAva1VWLXQIMoevqQn9us//C5TX9LEnBGH47D0zvJpjwjxb1umzgOMvE8Wg==", + "dev": true, + "license": "MIT", + "dependencies": { + "react-is": "^19.3.0", + "scheduler": "^0.28.0" + }, + "peerDependencies": { + "react": "^19.3.0" + } + }, + "node_modules/react-test-renderer/node_modules/react-is": { + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/react-is/-/react-is-19.3.0.tgz", + "integrity": "sha512-UpMYezM4v5/18F28aC66AEsjXIgE02kyEMH6yLdgLXu/UTfa1Ntwck/nNLrbqJsEXW7gPb0coNO9FQse9WTovA==", + "dev": true, + "license": "MIT" + }, "node_modules/readable-stream": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-4.7.0.tgz", @@ -13725,7 +13781,7 @@ "version": "4.23.15", "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.23.15.tgz", "integrity": "sha512-Yiex1Ovn8z2xPpOWckIiysV1SSyRMY9BkLF++q0yKiDxCqRhosKfMg3janKkiLBwZ5c/YryloKwGZcrEmtwxKw==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "esbuild": "~0.28.0" diff --git a/package.json b/package.json index 3020c50..8a9cb6a 100644 --- a/package.json +++ b/package.json @@ -58,11 +58,13 @@ "@types/node": "^26.6.3", "@types/react": "^19.3.0", "@types/react-dom": "^19.3.0", + "@types/react-test-renderer": "^19.3.0", "@vitejs/plugin-react": "^6.1.1", "concurrently": "^10.0.5", "eslint": "^10.11.0", "eslint-plugin-react-hooks": "^7.1.1", "prettier": "^3.9.9", + "react-test-renderer": "^19.3.0", "tsx": "^4.23.15", "typescript": "^6.0.3", "typescript-eslint": "^8.71.0", diff --git a/src/client/useVoice.ts b/src/client/useVoice.ts index 3909137..d57adc5 100644 --- a/src/client/useVoice.ts +++ b/src/client/useVoice.ts @@ -1,5 +1,9 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { api, authHeaders } from './api'; +// A single failed control poll is usually a network flap, not a dead call. +// Only treat the control connection as lost after this many consecutive +// poll failures, mirroring the grace period #26 gives the peer connection. +const CONTROL_POLL_FAILURE_LIMIT = 3; export function useVoice( threadId: string, onSaved: () => void, @@ -29,6 +33,7 @@ export function useVoice( channel: RTCDataChannel; transcript: string[]; timer?: ReturnType; + controlPollFailures: number; cancelled: boolean; } | undefined @@ -111,10 +116,18 @@ export function useVoice( if (id) void api<{ endedAt: number | null }>(`/voice/calls/${id}`) .then((call) => { - if (session.current === current && call.endedAt) void end(); + if (session.current !== current) return; + if (call.endedAt) { + void end(); + return; + } + current.controlPollFailures = 0; }) .catch(() => { if (session.current !== current) return; + current.controlPollFailures += 1; + if (current.controlPollFailures < CONTROL_POLL_FAILURE_LIMIT) + return; setError('Call control connection was lost.'); void end(); }); @@ -153,6 +166,7 @@ export function useVoice( cancelled: false, id: undefined as string | undefined, timer: undefined as ReturnType | undefined, + controlPollFailures: 0, }; session.current = current; stream.getTracks().forEach((track) => pc.addTrack(track, stream!)); diff --git a/tests/use-voice-control-poll.test.ts b/tests/use-voice-control-poll.test.ts new file mode 100644 index 0000000..2a3f540 --- /dev/null +++ b/tests/use-voice-control-poll.test.ts @@ -0,0 +1,125 @@ +import { createElement } from 'react'; +import { act, create, type ReactTestRenderer } from 'react-test-renderer'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import { useVoice } from '../src/client/useVoice'; +const api = vi.hoisted(() => vi.fn()); +vi.mock('../src/client/api', () => ({ api, authHeaders: () => ({}) })); +let voice: ReturnType; +let root: ReactTestRenderer; +let pc: FakePeer | undefined; +const track = { stop: vi.fn(), enabled: true }; +class FakePeer { + connectionState = 'new'; + onconnectionstatechange?: () => void; + channel = { close: vi.fn(), readyState: 'open' }; + constructor() { + // The test needs the peer created by the hook, like a browser constructor. + // eslint-disable-next-line @typescript-eslint/no-this-alias + pc = this; + } + createDataChannel() { + return this.channel; + } + addTrack() {} + async createOffer() { + return { sdp: 'offer' }; + } + async setLocalDescription() {} + async setRemoteDescription() {} + close() {} + state(value: string) { + this.connectionState = value; + this.onconnectionstatechange?.(); + } +} +function Hook() { + voice = useVoice('thread', () => {}); + return null; +} +function failPolls(times: number) { + let seen = 0; + api.mockImplementation(async (path: string) => { + if (path === '/voice/calls') return { id: 'call', sdp: 'answer' }; + if (path === '/voice/calls/call' && seen < times) { + seen += 1; + throw new Error('network flap'); + } + return { endedAt: null }; + }); +} +beforeEach(async () => { + vi.useFakeTimers(); + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); + vi.stubGlobal('RTCPeerConnection', FakePeer); + vi.stubGlobal('navigator', { + mediaDevices: { getUserMedia: async () => ({ getTracks: () => [track] }) }, + }); + vi.stubGlobal( + 'Audio', + class { + autoplay = false; + srcObject = null; + pause() {} + }, + ); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({})); + api + .mockReset() + .mockImplementation(async (path: string) => + path === '/voice/calls' + ? { id: 'call', sdp: 'answer' } + : { endedAt: null }, + ); + await act(async () => { + root = create(createElement(Hook)); + }); + await act(async () => { + await voice.start(); + pc?.state('connected'); + }); + expect(voice.status).toBe('active'); +}); +afterEach(async () => { + await act(async () => root.unmount()); + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); +const ends = () => api.mock.calls.filter(([path]) => path.endsWith('/end')); +it('keeps the call alive after a single failed control poll', async () => { + failPolls(1); + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + expect(ends()).toHaveLength(0); + expect(voice.status).toBe('active'); + expect(voice.error).toBe(''); +}); +it('keeps the call alive when a later poll succeeds before the failure limit', async () => { + failPolls(2); + await act(async () => { + await vi.advanceTimersByTimeAsync(6000); + }); + expect(ends()).toHaveLength(0); + failPolls(2); + await act(async () => { + await vi.advanceTimersByTimeAsync(6000); + }); + expect(ends()).toHaveLength(0); + expect(voice.status).toBe('active'); +}); +it('ends the call after three consecutive failed control polls', async () => { + failPolls(3); + await act(async () => { + await vi.advanceTimersByTimeAsync(6000); + }); + expect(ends()).toHaveLength(1); + expect(voice.status).toBe('idle'); + expect(voice.error).toBe('Call control connection was lost.'); +}); +it('ends a persistently failing control poll exactly once', async () => { + failPolls(10); + await act(async () => { + await vi.advanceTimersByTimeAsync(20000); + }); + expect(ends()).toHaveLength(1); +});