From 8a2eb4ab61f4adee2f2a28a6c38129333cc7ad1c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:36:02 +0800 Subject: [PATCH] fix(source-maps): preserve decoder lifecycle facts (#10069) * fix(source-maps): preserve decoder lifecycle facts * fix(source-maps): preserve preparation failure on cancellation * fix(source-maps): retain preparation and retry cleanup facts --- .../sourceMaps/SourceMapCommand.test.ts | 93 +++++++++ src/javascript/sourceMaps/SourceMapCommand.ts | 102 ++++++++-- .../sourceMaps/SourceMapDecoder.test.ts | 176 +++++++++++++++++- src/javascript/sourceMaps/SourceMapDecoder.ts | 29 ++- 4 files changed, 368 insertions(+), 32 deletions(-) diff --git a/src/javascript/sourceMaps/SourceMapCommand.test.ts b/src/javascript/sourceMaps/SourceMapCommand.test.ts index 4171464e40..a0184a30a5 100644 --- a/src/javascript/sourceMaps/SourceMapCommand.test.ts +++ b/src/javascript/sourceMaps/SourceMapCommand.test.ts @@ -1,7 +1,9 @@ +import { ProcessAdmissionError } from "../../process/ProcessAdmissionError.js"; import { access } from "node:fs/promises"; import { afterEach, expect, it, vi } from "vitest"; import { AnalysisCancelledError, + AnalysisOutputError, AnalysisResourceConstraintError, AnalysisTimeoutError, } from "../../domain/analysisErrorCore.js"; @@ -123,6 +125,67 @@ it("preserves Linux process inspection failure and recovery before launching the }); }); +it("retains a physical process inspection failure when the caller cancels concurrently", async () => { + const controller = new AbortController(); + const reason = { caller: "cancelled during inspection" }; + const physical = Object.assign(new Error("ps permission denied"), { + code: "EACCES", + }); + const host = createSystemProcessOwnershipHost( + "linux", + {}, + { + execFileOutput: async () => { + controller.abort(reason); + throw physical; + }, + }, + ); + const result = await runSourceMapCommand( + "{}", + "/probe/source.map", + { signal: controller.signal }, + { + prepareOwnership: () => host.prepare!(), + launcher: async () => { + throw new Error("must not launch"); + }, + }, + unexpectedCleanup, + ); + if (result.ok) throw new Error("expected inspection failure"); + expect(result.error).toBeInstanceOf(LinuxOwnedProcessPrerequisiteError); + expect(result.error.cause).toBe(physical); + expect(result.error.secondaryFailures).toHaveLength(1); + expect(result.error.secondaryFailures[0]).toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + }); +}); + +it("does not adopt an opaque typed cancellation reason as an ownership failure", async () => { + const controller = new AbortController(); + const reason = new AnalysisOutputError("unrelated", "opaque caller reason"); + const result = await runSourceMapCommand( + "{}", + "/probe/source.map", + { signal: controller.signal }, + { + prepareOwnership: async () => { + controller.abort(reason); + throw reason; + }, + launcher: async () => { + throw new Error("must not launch"); + }, + }, + unexpectedCleanup, + ); + if (result.ok) throw new Error("expected cancellation"); + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(reason); +}); + const exitsThenDescendantExceedsStderrBudget = 'const {spawn}=require("node:child_process"); spawn(process.execPath,["-e",`setTimeout(()=>{process.stderr.write("x".repeat(1024*1024+64)); setInterval(()=>{},1000)},25)`],{stdio:["ignore","inherit","inherit"]}); process.stderr.write("physical exit 7"); process.exit(7)'; @@ -270,6 +333,11 @@ it("preserves an observed nonzero exit before caller cancellation during output output_complete: false, }); expect(result.error.capturedOutput?.truncated).toBe(true); + expect( + result.error.secondaryFailures.find( + (failure) => failure instanceof AnalysisCancelledError, + )?.cause, + ).toBe(reason); if (runtimePath === undefined) throw new Error("runtime was not acquired"); await expect(access(runtimePath)).rejects.toMatchObject({ code: "ENOENT" }); }); @@ -446,3 +514,28 @@ it("applies a non-default decode deadline and releases its runtime", async () => vi.useRealTimers(); } }); + +it("retains a physical process admission error alongside cancellation", async () => { + const controller = new AbortController(); + const reason = { caller: "during admission" }; + const physical = new ProcessAdmissionError("reader_closed"); + const result = await runSourceMapCommand( + "{}", + "/probe/source.map", + { signal: controller.signal }, + { + prepareOwnership: async () => { + controller.abort(reason); + throw physical; + }, + }, + unexpectedCleanup, + ); + if (result.ok) throw new Error("expected admission failure"); + expect(result.error._tag).toBe("ProviderAdapterError"); + expect(result.error.cause).toBe(physical); + expect(result.error.secondaryFailures[0]).toBeInstanceOf( + AnalysisCancelledError, + ); + expect(result.error.secondaryFailures[0]?.cause).toBe(reason); +}); diff --git a/src/javascript/sourceMaps/SourceMapCommand.ts b/src/javascript/sourceMaps/SourceMapCommand.ts index 3a538086be..a1810bafb2 100644 --- a/src/javascript/sourceMaps/SourceMapCommand.ts +++ b/src/javascript/sourceMaps/SourceMapCommand.ts @@ -10,6 +10,7 @@ import { fileURLToPath } from "node:url"; import type { ExecutionOptions } from "../../application/AnalysisProvider.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js"; +import { analysisErrorWithCancellation } from "../../domain/analysisErrorCancellation.js"; import { AnalysisCancelledError, AnalysisOutputError, @@ -28,6 +29,8 @@ import { } from "../../domain/webSourceLocation.js"; import { parseSourceMapReply } from "./SourceMapReply.js"; import { PrivateRuntimeRoot } from "../../process/PrivateRuntimeRoot.js"; +import { DarwinProcessOwnershipInspectionError } from "../../process/DarwinProcessRunTokenReader.js"; +import { ProcessAdmissionError } from "../../process/ProcessAdmissionError.js"; import { RuntimeControlUnavailableError, runtimeControlCapabilityError, @@ -117,21 +120,46 @@ export const runSourceMapCommand = async ( options?.signal, ); } catch (cause: unknown) { + const signal = options?.signal; + const preparation = + cause !== signal?.reason && + cause instanceof DarwinProcessOwnershipInspectionError + ? cause + : undefined; + const knownPhysical = + cause !== signal?.reason && + (cause instanceof AnalysisError || + cause instanceof ProcessAdmissionError || + (preparation !== undefined && preparation.cancellation === undefined)); + const physical = + cause instanceof AnalysisError + ? cause + : new ProviderAdapterError(PROVIDER, OPERATION, { + cause, + ...(preparation?.capturedOutput === undefined + ? {} + : { capturedOutput: preparation.capturedOutput }), + diagnostics: { + map_path: path, + error_message: message(cause), + phase: "process_ownership_preparation", + }, + }); + const primary = signal?.aborted + ? analysisErrorWithCancellation( + knownPhysical ? physical : undefined, + new AnalysisCancelledError(OPERATION, { cause: signal.reason }), + ) + : physical; + if (preparation?.cleanupFailure === undefined) return err(primary); + const owner = new PreparationCleanup(preparation, path); + retainedCleanup.owner = owner; return err( - options?.signal?.aborted === true - ? new AnalysisCancelledError(OPERATION, { - cause: options.signal.reason, - }) - : cause instanceof AnalysisError - ? cause - : new ProviderAdapterError(PROVIDER, OPERATION, { - cause, - diagnostics: { - map_path: path, - error_message: message(cause), - phase: "process_ownership_preparation", - }, - }), + analysisErrorWithCleanupFailure( + primary, + preparation.cleanupFailure, + OPERATION, + ).bindCleanupDependencies([preparation.cleanupFailure], owner), ); } const deadline = new ProviderStartupDeadline( @@ -162,15 +190,17 @@ export const runSourceMapCommand = async ( } const cleanup = await command.close(); const observation = result.ok ? result.value : undefined; - const completed = - result.ok && options?.signal?.aborted - ? err( + const completed = options?.signal?.aborted + ? err( + analysisErrorWithCancellation( + result.ok ? undefined : result.error, new AnalysisCancelledError(OPERATION, { cause: options.signal.reason, partialObservation: observation, }), - ) - : result; + ), + ) + : result; if (cleanup === undefined) return completed; if (observation !== undefined) cleanup.retainPartialObservation(observation); retainedCleanup.owner = command; @@ -185,6 +215,40 @@ export const runSourceMapCommand = async ( ); }; +/** Keep the exact Darwin helper owner available after failed preparation cleanup. */ +class PreparationCleanup implements SourceMapCleanupOwner { + constructor( + readonly preparation: DarwinProcessOwnershipInspectionError, + readonly path: string, + ) {} + + get cleanupResources(): readonly string[] { + return this.preparation.cleanupFailure?.cleanupResources ?? []; + } + + async close(): Promise { + const cleanup = this.preparation.cleanupFailure; + if (cleanup === undefined || !cleanup.cleanupIncomplete) return undefined; + try { + if (this.preparation.cleanupOwner !== undefined) + await this.preparation.cleanupOwner.close(); + } catch (cause: unknown) { + return new SourceMapCleanupFailure(cleanup.cleanupResources, { + map_path: this.path, + cleanup_failures: [message(cause)], + }).bindCleanupDependencies([cleanup], this); + } + return cleanup.cleanupIncomplete + ? new SourceMapCleanupFailure(cleanup.cleanupResources, { + map_path: this.path, + cleanup_failures: [ + "Ownership preparation cleanup remains incomplete", + ], + }).bindCleanupDependencies([cleanup], this) + : undefined; + } +} + class OwnedCodec implements SourceMapCleanupOwner { readonly #runId = `rea-source-map-${randomUUID()}`; #runtime: SourceMapCodecRuntime | undefined; diff --git a/src/javascript/sourceMaps/SourceMapDecoder.test.ts b/src/javascript/sourceMaps/SourceMapDecoder.test.ts index 31c8eeb7cc..5b974be602 100644 --- a/src/javascript/sourceMaps/SourceMapDecoder.test.ts +++ b/src/javascript/sourceMaps/SourceMapDecoder.test.ts @@ -1,5 +1,7 @@ import { access } from "node:fs/promises"; import { expect, it } from "vitest"; +import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; +import { DarwinProcessOwnershipInspectionError } from "../../process/DarwinProcessRunTokenReader.js"; import { SourceMapDecoder } from "./SourceMapDecoder.js"; import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; @@ -35,19 +37,23 @@ it("preserves the exact caller reason before source-map admission", async () => expect(response.error.cause).toBe(reason); }); -it("retains failed cleanup ownership and retries it before another decode", async () => { +it("retains physical failure and late cancellation through cleanup failure and retry", async () => { const root = await PrivateRuntimeRoot.create({ prefix: "rea-source-map-cleanup-retry-test-", }); let cleanupAttempts = 0; + const controller = new AbortController(); + const reason = { caller: "after physical exit during final cleanup" }; const dependencies: SourceMapDecoderDependencies = { prepareOwnership: async () => {}, createRuntime: async () => ({ path: root.path, close: async () => { cleanupAttempts += 1; - if (cleanupAttempts === 1) + if (cleanupAttempts === 1) { + controller.abort(reason); throw new Error("injected root cleanup failure"); + } await root.close(); }, }), @@ -62,7 +68,7 @@ it("retains failed cleanup ownership and retries it before another decode", asyn }; const decoder = new SourceMapDecoder(dependencies); try { - const failed = await decoder.trace(input); + const failed = await decoder.trace(input, { signal: controller.signal }); expect(failed.ok).toBe(false); if (failed.ok) throw new Error("expected physical failure with cleanup failure"); @@ -75,6 +81,11 @@ it("retains failed cleanup ownership and retries it before another decode", asyn }, }); expect(primary?.capturedOutput?.stderr).toBe("physical exit 7"); + expect(primary?.secondaryFailures).toHaveLength(1); + expect(primary?.secondaryFailures[0]).toBeInstanceOf( + AnalysisCancelledError, + ); + expect(primary?.secondaryFailures[0]?.cause).toBe(reason); expect(failed.error.capturedOutput).toEqual(primary?.capturedOutput); expect(failed.error.cause).toBeInstanceOf(AggregateError); expect((failed.error.cause as AggregateError).errors[0]).toBe(primary); @@ -104,6 +115,8 @@ it("retains failed cleanup ownership and retries it before another decode", asyn expect(failed.error.cleanupResources).toEqual([]); expect(projectAnalysisError(failed.error).code).toBe("execution_failure"); expect(failed.error.cleanupPrimary).toBe(primary); + expect(primary?.secondaryFailures).toHaveLength(1); + expect(primary?.secondaryFailures[0]?.cause).toBe(reason); expect(failed.error.diagnostics).toMatchObject({ cleanup_error: { details: { @@ -241,3 +254,160 @@ it("attributes cleanup resources to the remaining process and directory owners", await decoder.close(); } }); + +it.each(["already closed", "queued at close"] as const)( + "reports source-map readiness without inventing cleanup ownership when %s", + async (phase) => { + const decoder = new SourceMapDecoder(); + const response = + phase === "queued at close" ? decoder.trace(input) : undefined; + await decoder.close(); + const result = await (response ?? decoder.trace(input)); + if (result.ok) throw new Error("Expected closed decoder refusal"); + expect(result.error._tag).toBe("AnalysisCapabilityUnavailableError"); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupResources).toEqual([]); + await decoder.close(); + }, +); + +it("retains cancellation while retrying an earlier failed cleanup", async () => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-source-map-retry-abort-", + }); + const controller = new AbortController(); + const reason = { caller: "during retry" }; + let attempts = 0; + let release = false; + const decoder = new SourceMapDecoder({ + prepareOwnership: async () => {}, + createRuntime: async () => ({ + path: root.path, + close: async () => { + attempts++; + if (!release) { + if (attempts === 2) controller.abort(reason); + throw new Error("root close failed"); + } + await root.close(); + }, + }), + launcher: async (options) => + spawnOwnedProviderProcess({ + ...options, + arguments: [ + "-e", + 'process.stderr.write("physical exit 7"); process.exit(7)', + ], + }), + }); + try { + const first = await decoder.trace(input); + if (first.ok) throw new Error("expected retained cleanup"); + const retry = await decoder.trace(input, { signal: controller.signal }); + if (retry.ok) throw new Error("expected retained cleanup on retry"); + expect(controller.signal.aborted).toBe(true); + const cancellation = retry.error.secondaryFailures.find( + (failure) => failure instanceof AnalysisCancelledError, + ); + expect(cancellation?.cause).toBe(reason); + expect(retry.error.cleanupIncomplete).toBe(true); + expect(retry.error.cleanupResources).toContain(root.path); + await expect(access(root.path)).resolves.toBeUndefined(); + release = true; + await decoder.close(); + await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); + expect(first.error.cleanupIncomplete).toBe(false); + } finally { + release = true; + await decoder.close(); + } +}); + +it("retains Darwin preparation cleanup ownership and cancellation through retry", async () => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-source-map-prep-owner-", + }); + const controller = new AbortController(); + const reason = { caller: "after preparation failure" }; + const cleanup = new ProviderCleanupError("darwin-helper", [root.path], { + reason: "held", + }); + let release = false; + const physical = new DarwinProcessOwnershipInspectionError( + "compiler failure", + { + cleanupFailure: cleanup, + cleanupOwner: { + close: async () => { + if (!release) throw new Error("still held"); + await root.close(); + cleanup.resolveCleanup(); + }, + }, + }, + ); + const decoder = new SourceMapDecoder({ + prepareOwnership: async () => { + controller.abort(reason); + throw physical; + }, + }); + try { + const failed = await decoder.trace(input, { signal: controller.signal }); + if (failed.ok) throw new Error("expected failed preparation"); + expect(failed.error.cleanupIncomplete).toBe(true); + expect(failed.error.cleanupResources).toContain(root.path); + expect(failed.error.cleanupPrimary?.cause).toBe(physical); + expect( + failed.error.cleanupPrimary?.secondaryFailures.find( + (entry) => entry instanceof AnalysisCancelledError, + )?.cause, + ).toBe(reason); + await expect(decoder.close()).rejects.toMatchObject({ + cleanupIncomplete: true, + }); + expect(failed.error.cleanupResources).toContain(root.path); + await expect(access(root.path)).resolves.toBeUndefined(); + release = true; + await decoder.close(); + await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); + expect(failed.error.cleanupIncomplete).toBe(false); + } finally { + release = true; + await decoder.close(); + await root.close(); + } +}); + +it("does not claim an opaque caller reason's Darwin cleanup owner", async () => { + const controller = new AbortController(); + const cleanup = new ProviderCleanupError("foreign", ["opaque-resource"], { + reason: "opaque", + }); + let ownerClosed = false; + const reason = new DarwinProcessOwnershipInspectionError( + "caller supplied reason", + { + cleanupFailure: cleanup, + cleanupOwner: { + close: async () => { + ownerClosed = true; + }, + }, + }, + ); + const decoder = new SourceMapDecoder({ + prepareOwnership: async () => { + controller.abort(reason); + throw reason; + }, + }); + const result = await decoder.trace(input, { signal: controller.signal }); + if (result.ok) throw new Error("expected cancellation"); + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(reason); + expect(result.error.cleanupIncomplete).toBe(false); + await decoder.close(); + expect(ownerClosed).toBe(false); +}); diff --git a/src/javascript/sourceMaps/SourceMapDecoder.ts b/src/javascript/sourceMaps/SourceMapDecoder.ts index d193ee4498..1ca387a5fc 100644 --- a/src/javascript/sourceMaps/SourceMapDecoder.ts +++ b/src/javascript/sourceMaps/SourceMapDecoder.ts @@ -6,8 +6,10 @@ import { import type { ExecutionOptions } from "../../application/AnalysisProvider.js"; import type { WebSourceMapPort } from "../../application/WebSourceLocationPorts.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; +import { analysisErrorWithCancellation } from "../../domain/analysisErrorCancellation.js"; import { AnalysisCancelledError, + AnalysisCapabilityUnavailableError, AnalysisInputError, } from "../../domain/analysisErrorCore.js"; import { ArtifactOperationError } from "../../domain/artifactOperationError.js"; @@ -23,7 +25,6 @@ import { type SourceMapCleanupRetention, type SourceMapDecoderDependencies, } from "./SourceMapCommand.js"; -import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; import { jsonParts } from "../../domain/jsonSerialization.js"; const OPERATION = "trace_web_source_location"; @@ -43,22 +44,20 @@ export class SourceMapDecoder implements WebSourceMapPort { if (this.#closing) return Promise.resolve( err( - new ProviderCleanupError( + new AnalysisCapabilityUnavailableError( "source-map-decoder", - [], - { reason: "Provider is closing" }, - { operation: OPERATION }, + OPERATION, + "Provider is closing", ), ), ); const operation = this.#tail.then(() => this.#closing ? err( - new ProviderCleanupError( + new AnalysisCapabilityUnavailableError( "source-map-decoder", - [], - { reason: "Provider is closing" }, - { operation: OPERATION }, + OPERATION, + "Provider is closing", ), ) : this.#trace(input, options), @@ -75,7 +74,17 @@ export class SourceMapDecoder implements WebSourceMapPort { options?: ExecutionOptions, ): Promise> { const cleanupFailure = await this.#retryCleanup(); - if (cleanupFailure !== undefined) return err(cleanupFailure); + if (cleanupFailure !== undefined) + return err( + options?.signal?.aborted + ? analysisErrorWithCancellation( + cleanupFailure, + new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + }), + ) + : cleanupFailure, + ); if (options?.signal?.aborted) return err( new AnalysisCancelledError(OPERATION, { cause: options.signal.reason }),