From 9d36c3437c305ec75756bc01c76d8f7f562e54e2 Mon Sep 17 00:00:00 2001 From: Jason Naylor Date: Wed, 2 Sep 2026 11:05:04 -0700 Subject: [PATCH 1/2] Guard the too-large check against a platform error with no message isPlatformError only tests for a platformErrorVersion property, so a value that passes it is not proven to carry a message even though PlatformError declares one as required. isPt9TooLargeError then read .includes off it. Every newPlatformError path does set message, and it is deliberately made enumerable so it survives the iframe boundary, so no platform producer reaches this. It is guarded anyway because the call sits inside a catch: a throw here escapes runPt9Import's handler and turns a failed import into an unhandled rejection rather than the reported failure the user should see. The RESOURCE_EXHAUSTED code check short-circuits first and is unaffected. The plain-Error branch needs nothing, since Error guarantees a string message through its prototype. Found by alex-rawlings-yyc while reviewing #274, who raised it as a robustness question rather than guessing at the producers. --- src/__tests__/utils/pt9-import-error.test.ts | 10 ++++++++++ src/utils/pt9-import-error.ts | 9 ++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/__tests__/utils/pt9-import-error.test.ts b/src/__tests__/utils/pt9-import-error.test.ts index deebc0c9..f6b9794b 100644 --- a/src/__tests__/utils/pt9-import-error.test.ts +++ b/src/__tests__/utils/pt9-import-error.test.ts @@ -28,4 +28,14 @@ describe('isPt9TooLargeError', () => { expect(isPt9TooLargeError(new Error('boom'))).toBe(false); expect(isPt9TooLargeError(MARKER_MESSAGE)).toBe(false); }); + + it('rejects a platform error carrying no message instead of throwing out of the catch', () => { + // isPlatformError only looks for platformErrorVersion, so this shape passes it while the + // declared message is absent. Reading .includes off undefined here would escape the catch + // in runPt9Import, turning a failed import into an unhandled rejection. + const noMessage: unknown = { platformErrorVersion: 1 }; + + expect(() => isPt9TooLargeError(noMessage)).not.toThrow(); + expect(isPt9TooLargeError(noMessage)).toBe(false); + }); }); diff --git a/src/utils/pt9-import-error.ts b/src/utils/pt9-import-error.ts index fd21d31e..a4e8e27f 100644 --- a/src/utils/pt9-import-error.ts +++ b/src/utils/pt9-import-error.ts @@ -13,6 +13,13 @@ const PT9_TOO_LARGE_MARKER = 'PT9 interlinear data is too large'; */ export function isPt9TooLargeError(error: unknown): boolean { if (isPlatformError(error)) - return error.code === 'RESOURCE_EXHAUSTED' || error.message.includes(PT9_TOO_LARGE_MARKER); + // isPlatformError only checks for platformErrorVersion, so a value that passes it is not + // proven to carry a message even though PlatformError declares one. Every newPlatformError + // path does set it, but this runs inside a catch: a throw here would escape runPt9Import's + // handler rather than being reported as a failed import. + return ( + error.code === 'RESOURCE_EXHAUSTED' || + (typeof error.message === 'string' && error.message.includes(PT9_TOO_LARGE_MARKER)) + ); return error instanceof Error && error.message.includes(PT9_TOO_LARGE_MARKER); } From 3b82a38fe3a30b39a2696797fb2f1347d2ad201c Mon Sep 17 00:00:00 2001 From: Jason Naylor Date: Wed, 2 Sep 2026 15:49:43 -0700 Subject: [PATCH 2/2] Rewrite the too-large guard comments to state the contract Trim the regression test's title to stop naming a catch it cannot show. The comments explained how isPlatformError narrows and named the function whose catch the call sits inside. Both go stale silently, and neither is what a reader of this function needs. What remains is the part the code cannot show: the narrowing does not prove the declared message is present, and the predicate answers rather than throws. --- src/__tests__/utils/pt9-import-error.test.ts | 6 ++---- src/utils/pt9-import-error.ts | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/__tests__/utils/pt9-import-error.test.ts b/src/__tests__/utils/pt9-import-error.test.ts index f6b9794b..90bc3b04 100644 --- a/src/__tests__/utils/pt9-import-error.test.ts +++ b/src/__tests__/utils/pt9-import-error.test.ts @@ -29,10 +29,8 @@ describe('isPt9TooLargeError', () => { expect(isPt9TooLargeError(MARKER_MESSAGE)).toBe(false); }); - it('rejects a platform error carrying no message instead of throwing out of the catch', () => { - // isPlatformError only looks for platformErrorVersion, so this shape passes it while the - // declared message is absent. Reading .includes off undefined here would escape the catch - // in runPt9Import, turning a failed import into an unhandled rejection. + it('rejects a platform error carrying no message instead of throwing', () => { + // Narrows as a platform error despite omitting the message the type declares as required. const noMessage: unknown = { platformErrorVersion: 1 }; expect(() => isPt9TooLargeError(noMessage)).not.toThrow(); diff --git a/src/utils/pt9-import-error.ts b/src/utils/pt9-import-error.ts index a4e8e27f..1b96cf86 100644 --- a/src/utils/pt9-import-error.ts +++ b/src/utils/pt9-import-error.ts @@ -13,10 +13,8 @@ const PT9_TOO_LARGE_MARKER = 'PT9 interlinear data is too large'; */ export function isPt9TooLargeError(error: unknown): boolean { if (isPlatformError(error)) - // isPlatformError only checks for platformErrorVersion, so a value that passes it is not - // proven to carry a message even though PlatformError declares one. Every newPlatformError - // path does set it, but this runs inside a catch: a throw here would escape runPt9Import's - // handler rather than being reported as a failed import. + // The narrowing proves less than the type does: a value can satisfy it without carrying the + // declared message, and we want to answer rather than throw. return ( error.code === 'RESOURCE_EXHAUSTED' || (typeof error.message === 'string' && error.message.includes(PT9_TOO_LARGE_MARKER))