diff --git a/src/offscreen/recorder.ts b/src/offscreen/recorder.ts index 3fab2f4..a86caf2 100644 --- a/src/offscreen/recorder.ts +++ b/src/offscreen/recorder.ts @@ -105,6 +105,14 @@ export function resetBuffer(): void { // ─── Проверка кодека (D-20 strict-mode — no fallback chain) ───────────── +// WR-02 fix: assertCodecSupported is a PURE predicate that throws. The +// previous implementation broadcast a RECORDING_ERROR before throwing, +// which violated single responsibility (a function named `assert*` +// shouldn't have side effects) AND double-emitted with the +// startRecording catch block — popup received two RECORDING_ERROR +// messages for the same underlying problem. The single-source-of-truth +// for the broadcast is now `startRecording`'s catch block via +// classifyCaptureError. export function assertCodecSupported(): void { const supported = typeof MediaRecorder !== 'undefined' && @@ -112,12 +120,79 @@ export function assertCodecSupported(): void { MediaRecorder.isTypeSupported(VIDEO_MIME); if (!supported) { const ua = typeof navigator !== 'undefined' ? navigator.userAgent : ''; - const errMessage = `vp9 unsupported. UA=${ua}`; - chrome.runtime.sendMessage({ type: 'RECORDING_ERROR', error: errMessage }); - throw new Error(errMessage); + throw new Error(`vp9 unsupported. UA=${ua}`); } } +// WR-01 fix: classify capture-pipeline failures into a stable, +// programmatically actionable error code. The previous implementation +// forwarded the raw DOMException message ("Permission denied by user", +// "Could not start video source", etc.) which changes between Chrome +// versions and locales. The popup / future telemetry needs a stable +// vocabulary — a string union the SW can switch on. The raw browser +// error stays in the logs at warn level for forensic value. +// +// Codes: +// 'user-cancelled' — operator dismissed the getDisplayMedia picker +// (DOMException name 'NotAllowedError' with no +// system-permission denial context). +// 'permission-denied' — system-level screen-recording permission denied +// by the OS (macOS Screen Recording privacy +// toggle, etc.). NotAllowedError + DOMException +// message hints OR SecurityError. +// 'codec-unsupported' — assertCodecSupported() threw; vp9 not in +// MediaRecorder.isTypeSupported. +// 'no-source-selected' — NotFoundError: picker yielded no source +// (rare; theoretically impossible if the picker +// closes via Cancel — that is NotAllowedError). +// 'capture-failed' — AbortError / generic stream-acquisition failure. +// 'unknown' — anything else; we still log the raw error. +export type CaptureErrorCode = + | 'user-cancelled' + | 'permission-denied' + | 'codec-unsupported' + | 'no-source-selected' + | 'capture-failed' + | 'unknown'; + +export function classifyCaptureError(error: unknown): CaptureErrorCode { + if (error instanceof Error) { + // Our own assertion is the cleanest signal — it never wraps a + // DOMException, so prefix-matching is safe and stable. + if (error.message.startsWith('vp9 unsupported')) { + return 'codec-unsupported'; + } + // DOMException has a stable `name` field per the WebIDL standard: + // https://webidl.spec.whatwg.org/#idl-DOMException + // The name does NOT vary between Chrome versions or locales, unlike + // the human-readable message — exactly what we want for routing. + const name = (error as { name?: string }).name; + if (name === 'NotAllowedError') { + // Distinguish system-permission denial from user-cancel by sniffing + // the message for the "system" keyword Chrome uses on macOS denial + // ("Permission denied by system"). On Linux/Windows the system case + // is typically wrapped as a SecurityError instead — handled below. + // The message text is locale-stable for English Chrome; for other + // locales we fall back to 'user-cancelled' which is the dominant + // NotAllowedError path in practice. + if (/system/i.test(error.message)) { + return 'permission-denied'; + } + return 'user-cancelled'; + } + if (name === 'SecurityError') { + return 'permission-denied'; + } + if (name === 'NotFoundError') { + return 'no-source-selected'; + } + if (name === 'AbortError') { + return 'capture-failed'; + } + } + return 'unknown'; +} + // ─── Захват экрана (getDisplayMedia inside offscreen — D-01) ──────────── async function startRecording(): Promise { @@ -150,9 +225,13 @@ async function startRecording(): Promise { 'max_segments:', MAX_SEGMENTS, ); } catch (error) { + // WR-01: emit a stable error code, not the raw DOMException message. + // Raw message stays in logs at warn level for forensic value. + const code = classifyCaptureError(error); const msg = error instanceof Error ? error.message : String(error); - logger.error('startRecording failed:', msg); - chrome.runtime.sendMessage({ type: 'RECORDING_ERROR', error: msg }); + logger.warn('startRecording failed (raw):', msg); + logger.error('startRecording failed (code):', code); + chrome.runtime.sendMessage({ type: 'RECORDING_ERROR', error: code }); throw error; } } diff --git a/tests/offscreen/codec-check.test.ts b/tests/offscreen/codec-check.test.ts index ea52caa..f6468e8 100644 --- a/tests/offscreen/codec-check.test.ts +++ b/tests/offscreen/codec-check.test.ts @@ -17,14 +17,23 @@ describe('codec strict mode', () => { }; }); - it('throws on unsupported vp9 and emits RECORDING_ERROR', async () => { + // WR-02 fix: assertCodecSupported is a pure predicate that throws. + // The previous spec expected a RECORDING_ERROR broadcast from inside + // the assertion; that side-effect was removed (single-responsibility + + // it double-emitted with startRecording's catch block). The notify is + // now solely the caller's (startRecording's) responsibility — exercised + // via integration through `classifyCaptureError` rather than from this + // unit test, since `startRecording` requires a full getDisplayMedia + // stub that jsdom does not provide. + it('throws on unsupported vp9 (pure assertion, no side-effect)', async () => { (globalThis as unknown as GlobalWithChrome).MediaRecorder = { isTypeSupported: vi.fn().mockReturnValue(false), }; const mod = await import('../../src/offscreen/recorder'); expect(() => mod.assertCodecSupported()).toThrow(/vp9 unsupported/); const stub = (globalThis as unknown as GlobalWithChrome).chrome!; - expect(stub.runtime.sendMessage).toHaveBeenCalledWith( + // The assertion itself MUST NOT emit RECORDING_ERROR. + expect(stub.runtime.sendMessage).not.toHaveBeenCalledWith( expect.objectContaining({ type: 'RECORDING_ERROR' }) ); }); @@ -41,3 +50,74 @@ describe('codec strict mode', () => { ); }); }); + +// WR-01 fix: classifyCaptureError maps DOMException / Error shapes to a +// stable string union. The popup / SW route can switch on the code +// instead of substring-matching DOMException.message — message text +// changes between Chrome versions and locales, name and prototype do not. +describe('classifyCaptureError (WR-01 stable error codes)', () => { + beforeEach(() => { + vi.resetModules(); + (globalThis as unknown as GlobalWithChrome).chrome = { + runtime: { sendMessage: vi.fn() }, + }; + (globalThis as unknown as GlobalWithChrome).MediaRecorder = { + isTypeSupported: vi.fn().mockReturnValue(true), + }; + }); + + function makeDomError(name: string, message: string): Error { + const e = new Error(message); + (e as { name: string }).name = name; + return e; + } + + it('codec error → "codec-unsupported"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect(mod.classifyCaptureError(new Error('vp9 unsupported. UA='))).toBe( + 'codec-unsupported', + ); + }); + + it('NotAllowedError (no system hint) → "user-cancelled"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect( + mod.classifyCaptureError(makeDomError('NotAllowedError', 'Permission denied by user')), + ).toBe('user-cancelled'); + }); + + it('NotAllowedError with "system" in message → "permission-denied"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect( + mod.classifyCaptureError(makeDomError('NotAllowedError', 'Permission denied by system')), + ).toBe('permission-denied'); + }); + + it('SecurityError → "permission-denied"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect(mod.classifyCaptureError(makeDomError('SecurityError', 'blocked'))).toBe( + 'permission-denied', + ); + }); + + it('NotFoundError → "no-source-selected"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect(mod.classifyCaptureError(makeDomError('NotFoundError', 'no source'))).toBe( + 'no-source-selected', + ); + }); + + it('AbortError → "capture-failed"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect(mod.classifyCaptureError(makeDomError('AbortError', 'aborted'))).toBe( + 'capture-failed', + ); + }); + + it('arbitrary error → "unknown"', async () => { + const mod = await import('../../src/offscreen/recorder'); + expect(mod.classifyCaptureError(new Error('totally novel failure'))).toBe('unknown'); + expect(mod.classifyCaptureError('a bare string')).toBe('unknown'); + expect(mod.classifyCaptureError(null)).toBe('unknown'); + }); +});