From 9e898ed4c9930b828e5687600a177904c9839b80 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 11:20:10 +0900 Subject: [PATCH 1/8] Preserve parse errors over disposal errors When parsing fails in runWith() or runWithSync() and a source context's disposal also throws, the disposal error used to silently replace the original parse error due to JavaScript's try/finally semantics. The disposal error now wraps both failures in a SuppressedError (following TC39 conventions) instead of discarding the parse error: - .error = the disposal error (the newer error) - .suppressed = the original parse error The try/finally blocks were restructured into helper functions (runWithBody, runWithSyncBody) with explicit error handling in the callers, avoiding the no-unsafe-finally lint issue entirely. https://github.com/dahlia/optique/issues/246 Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGES.md | 6 + packages/core/src/facade.test.ts | 242 ++++++++++++++++ packages/core/src/facade.ts | 480 ++++++++++++++++++------------- 3 files changed, 529 insertions(+), 199 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 98dc8ef21..edc78c570 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -358,6 +358,11 @@ To be released. multiple source contexts with the same `id`. Duplicate ids now throw a `TypeError` instead of producing order-dependent results. [[#495], [#746]] + - Fixed `runWith()` and `runWithSync()` discarding the original parse error + when a source context's disposal also throws. The disposal error now + wraps both failures in a `SuppressedError` (following TC39 conventions) + instead of silently replacing the parse error. [[#246]] + - Fixed `optional()` and `withDefault()` crashing when the parser's state is an annotation-injected object instead of `undefined`. The state discrimination in `modifiers.ts` now uses `Array.isArray(state)` to @@ -1182,6 +1187,7 @@ To be released. [#241]: https://github.com/dahlia/optique/issues/241 [#242]: https://github.com/dahlia/optique/issues/242 [#245]: https://github.com/dahlia/optique/issues/245 +[#246]: https://github.com/dahlia/optique/issues/246 [#247]: https://github.com/dahlia/optique/issues/247 [#248]: https://github.com/dahlia/optique/issues/248 [#249]: https://github.com/dahlia/optique/issues/249 diff --git a/packages/core/src/facade.test.ts b/packages/core/src/facade.test.ts index 31ffbd95a..73265a4c1 100644 --- a/packages/core/src/facade.test.ts +++ b/packages/core/src/facade.test.ts @@ -6300,6 +6300,127 @@ describe("runWith", () => { }); assert.equal(disposed, 1); }); + + it("should throw SuppressedError when parse fails and disposal also fails", async () => { + let disposed = false; + const context: SourceContext = { + id: Symbol.for("@test/dispose-shadow"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed = true; + throw new Error("dispose failed."); + }, + }; + + const parser = object({ + port: argument(integer()), + }); + + try { + await runWith(parser, "test", [context], { + args: ["not-a-number"], + }); + assert.fail("Expected an error to be thrown"); + } catch (error) { + assert.ok(error instanceof SuppressedError); + assert.ok( + error.suppressed instanceof Error, + "suppressed should be the parse error", + ); + assert.ok( + error.error instanceof Error, + "error should be the disposal error", + ); + assert.equal( + (error.error as Error).message, + "dispose failed.", + ); + } + + assert.ok(disposed); + }); + + it("should throw SuppressedError with AggregateError when parse fails and multiple disposals fail", async () => { + const disposed: string[] = []; + + const context1: SourceContext = { + id: Symbol.for("@test/dispose-shadow-multi-1"), + getAnnotations() { + return {}; + }, + [Symbol.asyncDispose]() { + disposed.push("context1"); + throw new Error("dispose 1 failed."); + }, + }; + + const context2: SourceContext = { + id: Symbol.for("@test/dispose-shadow-multi-2"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed.push("context2"); + throw new Error("dispose 2 failed."); + }, + }; + + const parser = object({ + port: argument(integer()), + }); + + try { + await runWith(parser, "test", [context1, context2], { + args: ["not-a-number"], + }); + assert.fail("Expected an error to be thrown"); + } catch (error) { + assert.ok(error instanceof SuppressedError); + assert.ok( + error.suppressed instanceof Error, + "suppressed should be the parse error", + ); + assert.ok( + error.error instanceof AggregateError, + "error should be AggregateError from multiple disposal failures", + ); + assert.equal((error.error as AggregateError).errors.length, 2); + } + + assert.deepEqual(disposed, ["context1", "context2"]); + }); + + it("should preserve parse error when disposal succeeds", async () => { + let disposed = false; + const context: SourceContext = { + id: Symbol.for("@test/dispose-no-shadow"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed = true; + }, + }; + + const parser = object({ + port: argument(integer()), + }); + + let errorCaught: unknown; + try { + await runWith(parser, "test", [context], { + args: ["not-a-number"], + }); + } catch (error) { + errorCaught = error; + } + + assert.ok(disposed); + assert.ok(!(errorCaught instanceof SuppressedError)); + assert.ok(errorCaught instanceof Error); + }); }); describe("options passthrough", () => { @@ -6997,6 +7118,127 @@ describe("runWithSync", () => { }); assert.equal(disposed, 1); }); + + it("should throw SuppressedError when sync parse fails and disposal also fails", () => { + let disposed = false; + const context: SourceContext = { + id: Symbol.for("@test/sync-dispose-shadow"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed = true; + throw new Error("sync dispose failed."); + }, + }; + + const parser = object({ + port: argument(integer()), + }); + + try { + runWithSync(parser, "test", [context], { + args: ["not-a-number"], + }); + assert.fail("Expected an error to be thrown"); + } catch (error) { + assert.ok(error instanceof SuppressedError); + assert.ok( + error.suppressed instanceof Error, + "suppressed should be the parse error", + ); + assert.ok( + error.error instanceof Error, + "error should be the disposal error", + ); + assert.equal( + (error.error as Error).message, + "sync dispose failed.", + ); + } + + assert.ok(disposed); + }); + + it("should throw SuppressedError with AggregateError when sync parse fails and multiple disposals fail", () => { + const disposed: string[] = []; + + const context1: SourceContext = { + id: Symbol.for("@test/sync-dispose-shadow-multi-1"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed.push("context1"); + throw new Error("sync dispose 1 failed."); + }, + }; + + const context2: SourceContext = { + id: Symbol.for("@test/sync-dispose-shadow-multi-2"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed.push("context2"); + throw new Error("sync dispose 2 failed."); + }, + }; + + const parser = object({ + port: argument(integer()), + }); + + try { + runWithSync(parser, "test", [context1, context2], { + args: ["not-a-number"], + }); + assert.fail("Expected an error to be thrown"); + } catch (error) { + assert.ok(error instanceof SuppressedError); + assert.ok( + error.suppressed instanceof Error, + "suppressed should be the parse error", + ); + assert.ok( + error.error instanceof AggregateError, + "error should be AggregateError from multiple disposal failures", + ); + assert.equal((error.error as AggregateError).errors.length, 2); + } + + assert.deepEqual(disposed, ["context1", "context2"]); + }); + + it("should preserve sync parse error when disposal succeeds", () => { + let disposed = false; + const context: SourceContext = { + id: Symbol.for("@test/sync-dispose-no-shadow"), + getAnnotations() { + return {}; + }, + [Symbol.dispose]() { + disposed = true; + }, + }; + + const parser = object({ + port: argument(integer()), + }); + + let errorCaught: unknown; + try { + runWithSync(parser, "test", [context], { + args: ["not-a-number"], + }); + } catch (error) { + errorCaught = error; + } + + assert.ok(disposed); + assert.ok(!(errorCaught instanceof SuppressedError)); + assert.ok(errorCaught instanceof Error); + }); }); describe("priority handling (sync)", () => { diff --git a/packages/core/src/facade.ts b/packages/core/src/facade.ts index f3b50755a..9602d3a17 100644 --- a/packages/core/src/facade.ts +++ b/packages/core/src/facade.ts @@ -3028,23 +3028,21 @@ export type ContextOptionsParam< * ); * ``` */ -export async function runWith< +async function runWithBody< TParser extends Parser, - TContexts extends readonly SourceContext[], THelp = void, TError = never, >( parser: TParser, programName: string, - contexts: TContexts, - options: - & RunWithOptions - & ContextOptionsParam>, + contexts: readonly SourceContext[], + args: readonly string[], + options: RunWithOptions, ): Promise> { - const args = options?.args ?? []; + validateContextIds(contexts); - // If no contexts, just run the parser directly - if (contexts.length === 0) { + // Early exit: skip context processing for help/version/completion + if (needsEarlyExit(args, options)) { if (parser.$mode === "async") { return runParser(parser, programName, args, options) as Promise< InferValue @@ -3055,150 +3053,281 @@ export async function runWith< ); } - try { - validateContextIds(contexts); - - // Early exit: skip context processing for help/version/completion - if (needsEarlyExit(args, options)) { - if (parser.$mode === "async") { - return runParser(parser, programName, args, options) as Promise< - InferValue - >; - } - return Promise.resolve( - runParser(parser, programName, args, options) as InferValue, - ); + // Phase 1: Collect initial annotations + const ctxOptions = (options as Record)?.contextOptions as + | Record + | undefined; + const { + annotations: phase1Annotations, + annotationsList: phase1AnnotationsList, + hasDynamic: needsTwoPhase, + } = await collectPhase1Annotations(contexts, ctxOptions); + + if (!needsTwoPhase) { + // All static contexts - single pass is sufficient + // Inject annotations into the parser's initial state + const augmentedParser = injectAnnotationsIntoParser( + parser, + phase1Annotations, + ); + + if (parser.$mode === "async") { + return runParser( + augmentedParser, + programName, + args, + options, + ) as Promise>; } + return Promise.resolve( + runParser(augmentedParser, programName, args, options) as InferValue< + TParser + >, + ); + } - // Phase 1: Collect initial annotations - const ctxOptions = options?.contextOptions; - const { - annotations: phase1Annotations, - annotationsList: phase1AnnotationsList, - hasDynamic: needsTwoPhase, - } = await collectPhase1Annotations(contexts, ctxOptions); - - if (!needsTwoPhase) { - // All static contexts - single pass is sufficient - // Inject annotations into the parser's initial state - const augmentedParser = injectAnnotationsIntoParser( - parser, - phase1Annotations, + // Two-phase parsing for dynamic contexts + // First pass: parse with Phase 1 annotations to get initial result + const augmentedParser1 = injectAnnotationsIntoParser( + parser, + phase1Annotations, + ); + + let firstPassResult: unknown; + let firstPassDeferred: true | undefined; + let firstPassDeferredKeys: DeferredMap | undefined; + let firstPassFailed = false; + try { + if (parser.$mode === "async") { + firstPassResult = await parseAsync(augmentedParser1, args); + } else { + firstPassResult = parseSync( + augmentedParser1 as Parser<"sync", unknown, unknown>, + args, ); + } - if (parser.$mode === "async") { - return runParser( - augmentedParser, - programName, - args, - options, - ) as Promise>; + // Extract value and deferred metadata from result + if ( + typeof firstPassResult === "object" && firstPassResult !== null && + "success" in firstPassResult + ) { + const result = firstPassResult as + & Result + & { deferred?: true; deferredKeys?: DeferredMap }; + if (result.success) { + firstPassResult = result.value; + firstPassDeferred = result.deferred; + firstPassDeferredKeys = result.deferredKeys; + } else { + firstPassFailed = true; } - return Promise.resolve( - runParser(augmentedParser, programName, args, options) as InferValue< - TParser - >, - ); } + } catch { + firstPassFailed = true; + } - // Two-phase parsing for dynamic contexts - // First pass: parse with Phase 1 annotations to get initial result - const augmentedParser1 = injectAnnotationsIntoParser( + // First pass failed - run through runParser for proper error handling. + // This is done outside the try-catch to prevent the catch block from + // re-invoking runParser when it throws (which caused double error output). + if (firstPassFailed) { + const augmentedParser = injectAnnotationsIntoParser( parser, phase1Annotations, ); - - let firstPassResult: unknown; - let firstPassDeferred: true | undefined; - let firstPassDeferredKeys: DeferredMap | undefined; - let firstPassFailed = false; - try { - if (parser.$mode === "async") { - firstPassResult = await parseAsync(augmentedParser1, args); - } else { - firstPassResult = parseSync( - augmentedParser1 as Parser<"sync", unknown, unknown>, - args, - ); - } - - // Extract value and deferred metadata from result - if ( - typeof firstPassResult === "object" && firstPassResult !== null && - "success" in firstPassResult - ) { - const result = firstPassResult as - & Result - & { deferred?: true; deferredKeys?: DeferredMap }; - if (result.success) { - firstPassResult = result.value; - firstPassDeferred = result.deferred; - firstPassDeferredKeys = result.deferredKeys; - } else { - firstPassFailed = true; - } - } - } catch { - firstPassFailed = true; + if (parser.$mode === "async") { + return runParser( + augmentedParser, + programName, + args, + options, + ) as Promise< + InferValue + >; } + return Promise.resolve( + runParser(augmentedParser, programName, args, options) as InferValue< + TParser + >, + ); + } - // First pass failed - run through runParser for proper error handling. - // This is done outside the try-catch to prevent the catch block from - // re-invoking runParser when it throws (which caused double error output). - if (firstPassFailed) { - const augmentedParser = injectAnnotationsIntoParser( - parser, - phase1Annotations, - ); - if (parser.$mode === "async") { - return runParser( - augmentedParser, - programName, - args, - options, - ) as Promise< - InferValue - >; - } - return Promise.resolve( - runParser(augmentedParser, programName, args, options) as InferValue< - TParser - >, - ); - } + // Phase 2: Collect annotations with parsed result + const { annotationsList: phase2AnnotationsList } = await collectAnnotations( + contexts, + firstPassResult, + ctxOptions, + firstPassDeferred, + firstPassDeferredKeys, + ); - // Phase 2: Collect annotations with parsed result - const { annotationsList: phase2AnnotationsList } = await collectAnnotations( - contexts, - firstPassResult, - ctxOptions, - firstPassDeferred, - firstPassDeferredKeys, - ); + // Final parse with merged annotations + const finalAnnotations = mergeTwoPhaseAnnotations( + phase1AnnotationsList, + phase2AnnotationsList, + ); + const augmentedParser2 = injectAnnotationsIntoParser( + parser, + finalAnnotations, + ); - // Final parse with merged annotations - const finalAnnotations = mergeTwoPhaseAnnotations( - phase1AnnotationsList, - phase2AnnotationsList, - ); - const augmentedParser2 = injectAnnotationsIntoParser( - parser, - finalAnnotations, - ); + if (parser.$mode === "async") { + return runParser(augmentedParser2, programName, args, options) as Promise< + InferValue + >; + } + return Promise.resolve( + runParser(augmentedParser2, programName, args, options) as InferValue< + TParser + >, + ); +} + +export async function runWith< + TParser extends Parser, + TContexts extends readonly SourceContext[], + THelp = void, + TError = never, +>( + parser: TParser, + programName: string, + contexts: TContexts, + options: + & RunWithOptions + & ContextOptionsParam>, +): Promise> { + const args = options?.args ?? []; + // If no contexts, just run the parser directly + if (contexts.length === 0) { if (parser.$mode === "async") { - return runParser(augmentedParser2, programName, args, options) as Promise< + return runParser(parser, programName, args, options) as Promise< InferValue >; } return Promise.resolve( - runParser(augmentedParser2, programName, args, options) as InferValue< - TParser - >, + runParser(parser, programName, args, options) as InferValue, ); - } finally { + } + + let result!: InferValue; + let primaryError: unknown; + let hasPrimaryError = false; + try { + result = await runWithBody(parser, programName, contexts, args, options); + } catch (error) { + hasPrimaryError = true; + primaryError = error; + } + + // Disposal runs unconditionally (success and error paths both reach here) + try { await disposeContexts(contexts); + } catch (disposeError) { + if (hasPrimaryError) { + throw new SuppressedError( + disposeError, + primaryError, + "An error was suppressed during context disposal.", + ); + } + throw disposeError; } + + if (hasPrimaryError) { + throw primaryError; + } + return result; +} + +/** + * Body of {@link runWithSync}, extracted so that the caller can handle + * disposal outside a `finally` block (avoiding `no-unsafe-finally` lint). + */ +function runWithSyncBody< + TParser extends Parser<"sync", unknown, unknown>, + THelp = void, + TError = never, +>( + parser: TParser, + programName: string, + contexts: readonly SourceContext[], + args: readonly string[], + options: RunWithOptions, +): InferValue { + validateContextIds(contexts); + + // Early exit: skip context processing for help/version/completion + if (needsEarlyExit(args, options)) { + return runParser(parser, programName, args, options); + } + + // Phase 1: Collect initial annotations + const ctxOptions = (options as Record)?.contextOptions as + | Record + | undefined; + const { + annotations: phase1Annotations, + annotationsList: phase1AnnotationsList, + hasDynamic: needsTwoPhase, + } = collectPhase1AnnotationsSync(contexts, ctxOptions); + + if (!needsTwoPhase) { + // All static contexts - single pass is sufficient + const augmentedParser = injectAnnotationsIntoParser( + parser, + phase1Annotations, + ); + return runParser(augmentedParser, programName, args, options); + } + + // Two-phase parsing for dynamic contexts + // First pass: parse with Phase 1 annotations + const augmentedParser1 = injectAnnotationsIntoParser( + parser, + phase1Annotations, + ); + + let firstPassResult: unknown; + let firstPassDeferred: true | undefined; + let firstPassDeferredKeys: DeferredMap | undefined; + try { + const result = parseSync(augmentedParser1, args) as + & { success: boolean; value?: unknown; error?: unknown } + & { deferred?: true; deferredKeys?: DeferredMap }; + if (result.success) { + firstPassResult = result.value; + firstPassDeferred = result.deferred; + firstPassDeferredKeys = result.deferredKeys; + } else { + // First pass failed - run through runParser for proper error handling + return runParser(augmentedParser1, programName, args, options); + } + } catch { + // First pass threw - run through runParser for proper error handling + return runParser(augmentedParser1, programName, args, options); + } + + // Phase 2: Collect annotations with parsed result + const { annotationsList: phase2AnnotationsList } = collectAnnotationsSync( + contexts, + firstPassResult, + ctxOptions, + firstPassDeferred, + firstPassDeferredKeys, + ); + + // Final parse with merged annotations + const finalAnnotations = mergeTwoPhaseAnnotations( + phase1AnnotationsList, + phase2AnnotationsList, + ); + const augmentedParser2 = injectAnnotationsIntoParser( + parser, + finalAnnotations, + ); + + return runParser(augmentedParser2, programName, args, options); } /** @@ -3250,81 +3379,34 @@ export function runWithSync< return runParser(parser, programName, args, options); } + let result!: InferValue; + let primaryError: unknown; + let hasPrimaryError = false; try { - validateContextIds(contexts); - - // Early exit: skip context processing for help/version/completion - if (needsEarlyExit(args, options)) { - return runParser(parser, programName, args, options); - } + result = runWithSyncBody(parser, programName, contexts, args, options); + } catch (error) { + hasPrimaryError = true; + primaryError = error; + } - // Phase 1: Collect initial annotations - const ctxOptions = options?.contextOptions; - const { - annotations: phase1Annotations, - annotationsList: phase1AnnotationsList, - hasDynamic: needsTwoPhase, - } = collectPhase1AnnotationsSync(contexts, ctxOptions); - - if (!needsTwoPhase) { - // All static contexts - single pass is sufficient - const augmentedParser = injectAnnotationsIntoParser( - parser, - phase1Annotations, + // Disposal runs unconditionally (success and error paths both reach here) + try { + disposeContextsSync(contexts); + } catch (disposeError) { + if (hasPrimaryError) { + throw new SuppressedError( + disposeError, + primaryError, + "An error was suppressed during context disposal.", ); - return runParser(augmentedParser, programName, args, options); - } - - // Two-phase parsing for dynamic contexts - // First pass: parse with Phase 1 annotations - const augmentedParser1 = injectAnnotationsIntoParser( - parser, - phase1Annotations, - ); - - let firstPassResult: unknown; - let firstPassDeferred: true | undefined; - let firstPassDeferredKeys: DeferredMap | undefined; - try { - const result = parseSync(augmentedParser1, args) as - & { success: boolean; value?: unknown; error?: unknown } - & { deferred?: true; deferredKeys?: DeferredMap }; - if (result.success) { - firstPassResult = result.value; - firstPassDeferred = result.deferred; - firstPassDeferredKeys = result.deferredKeys; - } else { - // First pass failed - run through runParser for proper error handling - return runParser(augmentedParser1, programName, args, options); - } - } catch { - // First pass threw - run through runParser for proper error handling - return runParser(augmentedParser1, programName, args, options); } + throw disposeError; + } - // Phase 2: Collect annotations with parsed result - const { annotationsList: phase2AnnotationsList } = collectAnnotationsSync( - contexts, - firstPassResult, - ctxOptions, - firstPassDeferred, - firstPassDeferredKeys, - ); - - // Final parse with merged annotations - const finalAnnotations = mergeTwoPhaseAnnotations( - phase1AnnotationsList, - phase2AnnotationsList, - ); - const augmentedParser2 = injectAnnotationsIntoParser( - parser, - finalAnnotations, - ); - - return runParser(augmentedParser2, programName, args, options); - } finally { - disposeContextsSync(contexts); + if (hasPrimaryError) { + throw primaryError; } + return result; } /** From cfb5bf33081880730add2bf8a42d0134502fd9f5 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 11:27:56 +0900 Subject: [PATCH 2/8] Add SuppressedError polyfill for Node < 24 SuppressedError is only a global starting from Node 24, but the package supports Node >= 20. Add a local polyfill so the double-failure disposal path works correctly on all supported runtimes instead of throwing ReferenceError. https://github.com/dahlia/optique/issues/246 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/facade.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/core/src/facade.ts b/packages/core/src/facade.ts index 9602d3a17..babf0c94b 100644 --- a/packages/core/src/facade.ts +++ b/packages/core/src/facade.ts @@ -66,6 +66,22 @@ import type { ParserValuePlaceholder, SourceContext } from "./context.ts"; export type { ParserValuePlaceholder, SourceContext }; +// Polyfill for runtimes that lack SuppressedError (Node < 24): +const SuppressedErrorCtor: typeof SuppressedError = + typeof SuppressedError === "function" ? SuppressedError : (() => { + class SuppressedErrorPolyfill extends Error { + error: unknown; + suppressed: unknown; + constructor(error: unknown, suppressed: unknown, message?: string) { + super(message); + this.name = "SuppressedError"; + this.error = error; + this.suppressed = suppressed; + } + } + return SuppressedErrorPolyfill as unknown as typeof SuppressedError; + })(); + function finalizeParsedForContext( context: SourceContext, parsed: unknown, @@ -3225,7 +3241,7 @@ export async function runWith< await disposeContexts(contexts); } catch (disposeError) { if (hasPrimaryError) { - throw new SuppressedError( + throw new SuppressedErrorCtor( disposeError, primaryError, "An error was suppressed during context disposal.", @@ -3394,7 +3410,7 @@ export function runWithSync< disposeContextsSync(contexts); } catch (disposeError) { if (hasPrimaryError) { - throw new SuppressedError( + throw new SuppressedErrorCtor( disposeError, primaryError, "An error was suppressed during context disposal.", From 797077471daec70a83e90d38b2eec9cf09f5a8d1 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 11:37:46 +0900 Subject: [PATCH 3/8] Add a PR link to the changelog --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index edc78c570..d78fe4022 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -361,7 +361,7 @@ To be released. - Fixed `runWith()` and `runWithSync()` discarding the original parse error when a source context's disposal also throws. The disposal error now wraps both failures in a `SuppressedError` (following TC39 conventions) - instead of silently replacing the parse error. [[#246]] + instead of silently replacing the parse error. [[#246], [#771]] - Fixed `optional()` and `withDefault()` crashing when the parser's state is an annotation-injected object instead of `undefined`. The state @@ -1457,6 +1457,7 @@ To be released. [#765]: https://github.com/dahlia/optique/pull/765 [#768]: https://github.com/dahlia/optique/issues/768 [#769]: https://github.com/dahlia/optique/pull/769 +[#771]: https://github.com/dahlia/optique/pull/771 ### @optique/config From afc78d8e6fad5b3bdc6a7d1cf02abca0b7cfb8c6 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 11:49:55 +0900 Subject: [PATCH 4/8] Use structural checks in SuppressedError tests Replace `instanceof SuppressedError` with duck-typing on `.name`/`.error`/`.suppressed` so the assertions don't depend on a global `SuppressedError` constructor. https://github.com/dahlia/optique/pull/771#discussion_r3034992274 https://github.com/dahlia/optique/pull/771#discussion_r3034994363 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/facade.test.ts | 44 +++++++++++++++++++------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/packages/core/src/facade.test.ts b/packages/core/src/facade.test.ts index 73265a4c1..ce732a5f8 100644 --- a/packages/core/src/facade.test.ts +++ b/packages/core/src/facade.test.ts @@ -6324,17 +6324,19 @@ describe("runWith", () => { }); assert.fail("Expected an error to be thrown"); } catch (error) { - assert.ok(error instanceof SuppressedError); + assert.ok(error instanceof Error); + assert.equal(error.name, "SuppressedError"); + const se = error as Error & { suppressed: unknown; error: unknown }; assert.ok( - error.suppressed instanceof Error, + se.suppressed instanceof Error, "suppressed should be the parse error", ); assert.ok( - error.error instanceof Error, + se.error instanceof Error, "error should be the disposal error", ); assert.equal( - (error.error as Error).message, + (se.error as Error).message, "dispose failed.", ); } @@ -6377,16 +6379,18 @@ describe("runWith", () => { }); assert.fail("Expected an error to be thrown"); } catch (error) { - assert.ok(error instanceof SuppressedError); + assert.ok(error instanceof Error); + assert.equal(error.name, "SuppressedError"); + const se = error as Error & { suppressed: unknown; error: unknown }; assert.ok( - error.suppressed instanceof Error, + se.suppressed instanceof Error, "suppressed should be the parse error", ); assert.ok( - error.error instanceof AggregateError, + se.error instanceof AggregateError, "error should be AggregateError from multiple disposal failures", ); - assert.equal((error.error as AggregateError).errors.length, 2); + assert.equal((se.error as AggregateError).errors.length, 2); } assert.deepEqual(disposed, ["context1", "context2"]); @@ -6418,7 +6422,7 @@ describe("runWith", () => { } assert.ok(disposed); - assert.ok(!(errorCaught instanceof SuppressedError)); + assert.notEqual((errorCaught as Error).name, "SuppressedError"); assert.ok(errorCaught instanceof Error); }); }); @@ -7142,17 +7146,19 @@ describe("runWithSync", () => { }); assert.fail("Expected an error to be thrown"); } catch (error) { - assert.ok(error instanceof SuppressedError); + assert.ok(error instanceof Error); + assert.equal(error.name, "SuppressedError"); + const se = error as Error & { suppressed: unknown; error: unknown }; assert.ok( - error.suppressed instanceof Error, + se.suppressed instanceof Error, "suppressed should be the parse error", ); assert.ok( - error.error instanceof Error, + se.error instanceof Error, "error should be the disposal error", ); assert.equal( - (error.error as Error).message, + (se.error as Error).message, "sync dispose failed.", ); } @@ -7195,16 +7201,18 @@ describe("runWithSync", () => { }); assert.fail("Expected an error to be thrown"); } catch (error) { - assert.ok(error instanceof SuppressedError); + assert.ok(error instanceof Error); + assert.equal(error.name, "SuppressedError"); + const se = error as Error & { suppressed: unknown; error: unknown }; assert.ok( - error.suppressed instanceof Error, + se.suppressed instanceof Error, "suppressed should be the parse error", ); assert.ok( - error.error instanceof AggregateError, + se.error instanceof AggregateError, "error should be AggregateError from multiple disposal failures", ); - assert.equal((error.error as AggregateError).errors.length, 2); + assert.equal((se.error as AggregateError).errors.length, 2); } assert.deepEqual(disposed, ["context1", "context2"]); @@ -7236,7 +7244,7 @@ describe("runWithSync", () => { } assert.ok(disposed); - assert.ok(!(errorCaught instanceof SuppressedError)); + assert.notEqual((errorCaught as Error).name, "SuppressedError"); assert.ok(errorCaught instanceof Error); }); }); From b4ee4c182a77f515cb5cc3a0d8f0aaa47c84b7a4 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 11:50:07 +0900 Subject: [PATCH 5/8] Simplify contextOptions access in body helpers Remove redundant casts; RunWithOptions already declares contextOptions as an optional property. https://github.com/dahlia/optique/pull/771#discussion_r3034994851 https://github.com/dahlia/optique/pull/771#discussion_r3034994853 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/facade.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/core/src/facade.ts b/packages/core/src/facade.ts index babf0c94b..5bf2d9f91 100644 --- a/packages/core/src/facade.ts +++ b/packages/core/src/facade.ts @@ -3070,9 +3070,7 @@ async function runWithBody< } // Phase 1: Collect initial annotations - const ctxOptions = (options as Record)?.contextOptions as - | Record - | undefined; + const ctxOptions = options.contextOptions; const { annotations: phase1Annotations, annotationsList: phase1AnnotationsList, @@ -3279,9 +3277,7 @@ function runWithSyncBody< } // Phase 1: Collect initial annotations - const ctxOptions = (options as Record)?.contextOptions as - | Record - | undefined; + const ctxOptions = options.contextOptions; const { annotations: phase1Annotations, annotationsList: phase1AnnotationsList, From 4d8b8f8436d36ee168e8e3a84a55a43cf0628d0b Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 14:03:12 +0900 Subject: [PATCH 6/8] Move JSDoc onto exported runWith/runWithSync The extraction left the public API docblock on the private runWithBody helper while the exported runWith was undocumented. Move it back and add @throws {SuppressedError} to both runWith and runWithSync. https://github.com/dahlia/optique/pull/771#discussion_r3035002791 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/facade.ts | 106 ++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 48 deletions(-) diff --git a/packages/core/src/facade.ts b/packages/core/src/facade.ts index 5bf2d9f91..aaae5b29d 100644 --- a/packages/core/src/facade.ts +++ b/packages/core/src/facade.ts @@ -2995,54 +2995,8 @@ export type ContextOptionsParam< : { readonly contextOptions: ExtractRequiredOptions }; /** - * Runs a parser with multiple source contexts. - * - * This function automatically handles static and dynamic contexts with proper - * priority. Earlier contexts in the array override later ones. - * - * The function uses a smart two-phase approach: - * - * 1. *Phase 1*: Collect annotations from all contexts (static contexts return - * their data, dynamic contexts may return empty). - * 2. *First parse*: Parse with Phase 1 annotations. - * 3. *Phase 2*: Call `getAnnotations(parsed)` on all contexts with the first - * parse result. - * 4. *Second parse*: Parse again with merged annotations from both phases. - * - * If all contexts are static (no dynamic contexts), the second parse is skipped - * for optimization. - * - * @template TParser The parser type. - * @template THelp Return type when help is shown. - * @template TError Return type when an error occurs. - * @param parser The parser to execute. - * @param programName Name of the program for help/error output. - * @param contexts Source contexts to use (priority: earlier overrides later). - * @param options Run options including args, help, version, etc. - * @returns Promise that resolves to the parsed result. - * @throws {TypeError} If two or more contexts share the same - * {@link SourceContext.id}. - * @since 0.10.0 - * - * @example - * ```typescript - * import { runWith } from "@optique/core/facade"; - * import type { SourceContext } from "@optique/core/context"; - * - * const envContext: SourceContext = { - * id: Symbol.for("@myapp/env"), - * getAnnotations() { - * return { [Symbol.for("@myapp/env")]: process.env }; - * } - * }; - * - * const result = await runWith( - * parser, - * "myapp", - * [envContext], - * { args: process.argv.slice(2) } - * ); - * ``` + * Body of {@link runWith}, extracted so that the caller can handle + * disposal outside a `finally` block (avoiding `no-unsafe-finally` lint). */ async function runWithBody< TParser extends Parser, @@ -3197,6 +3151,59 @@ async function runWithBody< ); } +/** + * Runs a parser with multiple source contexts. + * + * This function automatically handles static and dynamic contexts with proper + * priority. Earlier contexts in the array override later ones. + * + * The function uses a smart two-phase approach: + * + * 1. *Phase 1*: Collect annotations from all contexts (static contexts return + * their data, dynamic contexts may return empty). + * 2. *First parse*: Parse with Phase 1 annotations. + * 3. *Phase 2*: Call `getAnnotations(parsed)` on all contexts with the first + * parse result. + * 4. *Second parse*: Parse again with merged annotations from both phases. + * + * If all contexts are static (no dynamic contexts), the second parse is skipped + * for optimization. + * + * @template TParser The parser type. + * @template THelp Return type when help is shown. + * @template TError Return type when an error occurs. + * @param parser The parser to execute. + * @param programName Name of the program for help/error output. + * @param contexts Source contexts to use (priority: earlier overrides later). + * @param options Run options including args, help, version, etc. + * @returns Promise that resolves to the parsed result. + * @throws {TypeError} If two or more contexts share the same + * {@link SourceContext.id}. + * @throws {SuppressedError} If parsing fails and a context's disposal also + * throws. The original parse error is available via `.suppressed` and the + * disposal error via `.error`. + * @since 0.10.0 + * + * @example + * ```typescript + * import { runWith } from "@optique/core/facade"; + * import type { SourceContext } from "@optique/core/context"; + * + * const envContext: SourceContext = { + * id: Symbol.for("@myapp/env"), + * getAnnotations() { + * return { [Symbol.for("@myapp/env")]: process.env }; + * } + * }; + * + * const result = await runWith( + * parser, + * "myapp", + * [envContext], + * { args: process.argv.slice(2) } + * ); + * ``` + */ export async function runWith< TParser extends Parser, TContexts extends readonly SourceContext[], @@ -3362,6 +3369,9 @@ function runWithSyncBody< * {@link SourceContext.id}. * @throws {Error} If any context returns a Promise or if a context's * `[Symbol.asyncDispose]` returns a Promise. + * @throws {SuppressedError} If parsing fails and a context's disposal also + * throws. The original parse error is available via `.suppressed` and the + * disposal error via `.error`. * @since 0.10.0 */ export function runWithSync< From badbb89aa0c0592b222ba4433b9b1529bbe29331 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 14:50:25 +0900 Subject: [PATCH 7/8] Use structural type for SuppressedError polyfill Replace the `as unknown as typeof SuppressedError` double assertion with a local SuppressedErrorConstructor type that describes the polyfill's actual shape. Also add the missing @throws {SuppressedError} to runWithAsync() JSDoc. https://github.com/dahlia/optique/pull/771#discussion_r3035151281 https://github.com/dahlia/optique/pull/771#discussion_r3035151290 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/facade.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/core/src/facade.ts b/packages/core/src/facade.ts index aaae5b29d..4fb468516 100644 --- a/packages/core/src/facade.ts +++ b/packages/core/src/facade.ts @@ -66,12 +66,18 @@ import type { ParserValuePlaceholder, SourceContext } from "./context.ts"; export type { ParserValuePlaceholder, SourceContext }; +type SuppressedErrorConstructor = new ( + error: unknown, + suppressed: unknown, + message?: string, +) => Error & { readonly error: unknown; readonly suppressed: unknown }; + // Polyfill for runtimes that lack SuppressedError (Node < 24): -const SuppressedErrorCtor: typeof SuppressedError = +const SuppressedErrorCtor: SuppressedErrorConstructor = typeof SuppressedError === "function" ? SuppressedError : (() => { class SuppressedErrorPolyfill extends Error { - error: unknown; - suppressed: unknown; + readonly error: unknown; + readonly suppressed: unknown; constructor(error: unknown, suppressed: unknown, message?: string) { super(message); this.name = "SuppressedError"; @@ -79,7 +85,7 @@ const SuppressedErrorCtor: typeof SuppressedError = this.suppressed = suppressed; } } - return SuppressedErrorPolyfill as unknown as typeof SuppressedError; + return SuppressedErrorPolyfill; })(); function finalizeParsedForContext( @@ -3447,6 +3453,9 @@ export function runWithSync< * @returns Promise that resolves to the parsed result. * @throws {TypeError} If two or more contexts share the same * {@link SourceContext.id}. + * @throws {SuppressedError} If parsing fails and a context's disposal also + * throws. The original parse error is available via `.suppressed` and the + * disposal error via `.error`. * @since 0.10.0 */ export function runWithAsync< From 73dd4e3b6a2bc990b63832caf4a4018150abd275 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 4 Apr 2026 15:02:04 +0900 Subject: [PATCH 8/8] Broaden @throws {SuppressedError} wording The SuppressedError wraps any exception from the body (not just parse failures), so say "the runner throws" instead of "parsing fails". https://github.com/dahlia/optique/pull/771#discussion_r3035190161 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/facade.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/facade.ts b/packages/core/src/facade.ts index 4fb468516..272d439d0 100644 --- a/packages/core/src/facade.ts +++ b/packages/core/src/facade.ts @@ -3185,8 +3185,8 @@ async function runWithBody< * @returns Promise that resolves to the parsed result. * @throws {TypeError} If two or more contexts share the same * {@link SourceContext.id}. - * @throws {SuppressedError} If parsing fails and a context's disposal also - * throws. The original parse error is available via `.suppressed` and the + * @throws {SuppressedError} If the runner throws and a context's disposal + * also throws. The original error is available via `.suppressed` and the * disposal error via `.error`. * @since 0.10.0 * @@ -3375,8 +3375,8 @@ function runWithSyncBody< * {@link SourceContext.id}. * @throws {Error} If any context returns a Promise or if a context's * `[Symbol.asyncDispose]` returns a Promise. - * @throws {SuppressedError} If parsing fails and a context's disposal also - * throws. The original parse error is available via `.suppressed` and the + * @throws {SuppressedError} If the runner throws and a context's disposal + * also throws. The original error is available via `.suppressed` and the * disposal error via `.error`. * @since 0.10.0 */ @@ -3453,8 +3453,8 @@ export function runWithSync< * @returns Promise that resolves to the parsed result. * @throws {TypeError} If two or more contexts share the same * {@link SourceContext.id}. - * @throws {SuppressedError} If parsing fails and a context's disposal also - * throws. The original parse error is available via `.suppressed` and the + * @throws {SuppressedError} If the runner throws and a context's disposal + * also throws. The original error is available via `.suppressed` and the * disposal error via `.error`. * @since 0.10.0 */