mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(source-maps): retain primary failures through cleanup retries
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { access } from "node:fs/promises";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import {
|
||||
AnalysisCancelledError,
|
||||
AnalysisResourceConstraintError,
|
||||
@@ -14,8 +14,18 @@ import { WEB_SOURCE_MAP_LIMITS } from "../../domain/webSourceLocation.js";
|
||||
import {
|
||||
runSourceMapCommand,
|
||||
SOURCE_MAP_CODEC_MEMORY_LIMIT_BYTES,
|
||||
type SourceMapCleanupRetention,
|
||||
} from "./SourceMapCommand.js";
|
||||
|
||||
const unexpectedCleanup: SourceMapCleanupRetention = {};
|
||||
afterEach(async () => {
|
||||
const owner = unexpectedCleanup.owner;
|
||||
const failure = await owner?.close();
|
||||
if (failure !== undefined) throw failure;
|
||||
unexpectedCleanup.owner = undefined;
|
||||
expect(owner).toBeUndefined();
|
||||
});
|
||||
|
||||
const runChild = (
|
||||
signal: AbortSignal | undefined,
|
||||
script: string,
|
||||
@@ -44,6 +54,7 @@ const runChild = (
|
||||
return child;
|
||||
},
|
||||
},
|
||||
unexpectedCleanup,
|
||||
);
|
||||
|
||||
const exitsAfterHoldingOutput =
|
||||
@@ -65,6 +76,7 @@ it("preserves the exact object reason for an already-aborted caller", async () =
|
||||
throw new Error("must not launch");
|
||||
},
|
||||
},
|
||||
unexpectedCleanup,
|
||||
);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error("expected cancellation");
|
||||
@@ -72,6 +84,67 @@ it("preserves the exact object reason for an already-aborted caller", async () =
|
||||
expect(result.error.cause).toBe(reason);
|
||||
});
|
||||
|
||||
it.each([null, { caller: "late source-map cancellation" }])(
|
||||
"retains a late caller reason %j when runtime cleanup first fails",
|
||||
async (reason) => {
|
||||
const root = await PrivateRuntimeRoot.create({
|
||||
prefix: "rea-source-map-late-cleanup-test-",
|
||||
});
|
||||
const controller = new AbortController();
|
||||
let cleanupAttempts = 0;
|
||||
const retainedCleanup: SourceMapCleanupRetention = {};
|
||||
try {
|
||||
const result = await runSourceMapCommand(
|
||||
"{}",
|
||||
"/probe/source.map",
|
||||
{ signal: controller.signal },
|
||||
{
|
||||
prepareOwnership: async () => {},
|
||||
createRuntime: async () => ({
|
||||
path: root.path,
|
||||
close: async () => {
|
||||
cleanupAttempts += 1;
|
||||
if (cleanupAttempts === 1) {
|
||||
controller.abort(reason);
|
||||
throw new Error("controlled late cleanup failure");
|
||||
}
|
||||
await root.close();
|
||||
},
|
||||
}),
|
||||
launcher: async (options) =>
|
||||
spawnOwnedProviderProcess({
|
||||
...options,
|
||||
arguments: ["-e", 'process.stdout.write("complete")'],
|
||||
}),
|
||||
},
|
||||
retainedCleanup,
|
||||
);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok)
|
||||
throw new Error("expected cancellation with cleanup failure");
|
||||
expect(result.error.cleanupPrimary).toBeInstanceOf(
|
||||
AnalysisCancelledError,
|
||||
);
|
||||
expect(result.error.cleanupPrimary?.cause).toBe(reason);
|
||||
expect(result.error.cleanupResources).toEqual([root.path]);
|
||||
expect(result.error.cleanupIncomplete).toBe(true);
|
||||
expect(retainedCleanup.owner).toBeDefined();
|
||||
await retainedCleanup.owner?.close();
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(result.error.cleanupResources).toEqual([]);
|
||||
expect(result.error.cleanupPrimary?.cause).toBe(reason);
|
||||
} finally {
|
||||
await retainedCleanup.owner?.close();
|
||||
await root.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("returns completed codec output after successful cleanup", async () => {
|
||||
const result = await runChild(undefined, 'process.stdout.write("complete")');
|
||||
expect(result).toEqual({ ok: true, value: "complete" });
|
||||
});
|
||||
|
||||
it("preserves exact caller reason when cancellation precedes process exit", async () => {
|
||||
const reason = { caller: "active source-map cancellation" };
|
||||
const controller = new AbortController();
|
||||
@@ -222,6 +295,7 @@ it("preserves an observed process-memory limit over the codec exit status", asyn
|
||||
return spawned;
|
||||
},
|
||||
},
|
||||
unexpectedCleanup,
|
||||
);
|
||||
} finally {
|
||||
waitSpy.mockRestore();
|
||||
@@ -266,6 +340,7 @@ it("keeps a timeout distinct from caller cancellation", async () => {
|
||||
throw new Error("launcher settled after deadline");
|
||||
},
|
||||
},
|
||||
unexpectedCleanup,
|
||||
);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error("expected timeout");
|
||||
@@ -301,6 +376,7 @@ it("applies a non-default decode deadline and releases its runtime", async () =>
|
||||
throw new Error("launcher settled after deadline");
|
||||
},
|
||||
},
|
||||
unexpectedCleanup,
|
||||
250,
|
||||
);
|
||||
expect(result.ok).toBe(false);
|
||||
|
||||
@@ -4,6 +4,7 @@ import { join } from "node:path";
|
||||
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 {
|
||||
AnalysisCancelledError,
|
||||
AnalysisOutputError,
|
||||
@@ -62,9 +63,12 @@ export interface SourceMapDecoderDependencies {
|
||||
}
|
||||
|
||||
export interface SourceMapCleanupOwner {
|
||||
close(
|
||||
previousError: string | null,
|
||||
): Promise<ProviderCleanupError | undefined>;
|
||||
close(): Promise<ProviderCleanupError | undefined>;
|
||||
}
|
||||
|
||||
/** The caller retains retry authority independently of the returned error. */
|
||||
export interface SourceMapCleanupRetention {
|
||||
owner?: SourceMapCleanupOwner | undefined;
|
||||
}
|
||||
|
||||
type CodecPhysicalObservation =
|
||||
@@ -88,7 +92,6 @@ type CodecOutcome =
|
||||
|
||||
export class SourceMapCleanupFailure extends ProviderCleanupError {
|
||||
constructor(
|
||||
readonly cleanupOwner: SourceMapCleanupOwner,
|
||||
resources: readonly string[],
|
||||
diagnostics: Readonly<Record<string, JsonValue>>,
|
||||
) {
|
||||
@@ -102,6 +105,7 @@ export const runSourceMapCommand = async (
|
||||
path: string,
|
||||
options: ExecutionOptions | undefined,
|
||||
dependencies: SourceMapDecoderDependencies,
|
||||
retainedCleanup: SourceMapCleanupRetention,
|
||||
decodeTimeoutMs: number = DEFAULT_WEB_SOURCE_MAP_DECODE_TIMEOUT_MS,
|
||||
): Promise<Result<string, AnalysisError>> => {
|
||||
try {
|
||||
@@ -141,13 +145,26 @@ export const runSourceMapCommand = async (
|
||||
command.dispose();
|
||||
deadline.dispose();
|
||||
}
|
||||
const cleanup = await command.close(result.ok ? null : result.error.message);
|
||||
if (cleanup !== undefined) return err(cleanup);
|
||||
return result.ok && options?.signal?.aborted
|
||||
? err(
|
||||
new AnalysisCancelledError(OPERATION, { cause: options.signal.reason }),
|
||||
)
|
||||
: result;
|
||||
const cleanup = await command.close();
|
||||
const completed =
|
||||
result.ok && options?.signal?.aborted
|
||||
? err(
|
||||
new AnalysisCancelledError(OPERATION, {
|
||||
cause: options.signal.reason,
|
||||
}),
|
||||
)
|
||||
: result;
|
||||
if (cleanup === undefined) return completed;
|
||||
retainedCleanup.owner = command;
|
||||
return completed.ok
|
||||
? err(cleanup)
|
||||
: err(
|
||||
analysisErrorWithCleanupFailure(
|
||||
completed.error,
|
||||
cleanup,
|
||||
OPERATION,
|
||||
).bindCleanupDependencies([cleanup], command),
|
||||
);
|
||||
};
|
||||
|
||||
class OwnedCodec implements SourceMapCleanupOwner {
|
||||
@@ -471,9 +488,7 @@ class OwnedCodec implements SourceMapCleanupOwner {
|
||||
];
|
||||
}
|
||||
|
||||
async close(
|
||||
previousError: string | null,
|
||||
): Promise<ProviderCleanupError | undefined> {
|
||||
async close(): Promise<ProviderCleanupError | undefined> {
|
||||
const failures: string[] = [];
|
||||
let processReleased = this.#supervisor === undefined;
|
||||
if (this.#supervisor !== undefined) {
|
||||
@@ -497,10 +512,9 @@ class OwnedCodec implements SourceMapCleanupOwner {
|
||||
this.#cleanupAnchor = undefined;
|
||||
return undefined;
|
||||
}
|
||||
const failure = new SourceMapCleanupFailure(this, this.cleanupResources, {
|
||||
const failure = new SourceMapCleanupFailure(this.cleanupResources, {
|
||||
map_path: this.path,
|
||||
cleanup_failures: failures,
|
||||
previous_error: previousError,
|
||||
codec_pid: this.#supervisor?.launch.process.pid ?? null,
|
||||
}).bindCleanupResourceOwner(this);
|
||||
if (this.#cleanupAnchor === undefined) this.#cleanupAnchor = failure;
|
||||
|
||||
@@ -67,19 +67,30 @@ it("retains failed cleanup ownership and retries it before another decode", asyn
|
||||
expect(failed.ok).toBe(false);
|
||||
if (failed.ok)
|
||||
throw new Error("expected physical failure with cleanup failure");
|
||||
expect(failed.error).toBeInstanceOf(SourceMapCleanupFailure);
|
||||
if (!(failed.error instanceof SourceMapCleanupFailure))
|
||||
throw new Error("expected source-map cleanup ownership");
|
||||
expect(failed.error.cleanupOwner).toBeDefined();
|
||||
const primary = failed.error.cleanupPrimary;
|
||||
expect(primary).toMatchObject({
|
||||
_tag: "ProviderAdapterError",
|
||||
diagnostics: {
|
||||
exit_code: 7,
|
||||
stderr: "physical exit 7",
|
||||
},
|
||||
});
|
||||
expect(primary?.capturedOutput?.stderr).toBe("physical exit 7");
|
||||
expect(failed.error.capturedOutput).toEqual(primary?.capturedOutput);
|
||||
expect(failed.error.cause).toBeInstanceOf(AggregateError);
|
||||
expect((failed.error.cause as AggregateError).errors[0]).toBe(primary);
|
||||
expect(failed.error.cleanupResources).toEqual([root.path]);
|
||||
expect(failed.error.diagnostics).toMatchObject({
|
||||
cleanup_failures: ["injected root cleanup failure"],
|
||||
});
|
||||
expect(failed.error.diagnostics).toMatchObject({
|
||||
previous_error: expect.stringContaining(
|
||||
"source-map-decoder adapter failed",
|
||||
),
|
||||
primary_error: { code: "execution_failure" },
|
||||
cleanup_error: {
|
||||
details: {
|
||||
diagnostics: {
|
||||
cleanup_failures: ["injected root cleanup failure"],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(projectAnalysisError(failed.error).code).toBe("cleanup_incomplete");
|
||||
await expect(access(root.path)).resolves.toBeUndefined();
|
||||
|
||||
const invalidInput = { ...input, text: null } as unknown as typeof input;
|
||||
@@ -92,11 +103,16 @@ it("retains failed cleanup ownership and retries it before another decode", asyn
|
||||
await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(failed.error.cleanupIncomplete).toBe(false);
|
||||
expect(failed.error.cleanupResources).toEqual([]);
|
||||
expect(projectAnalysisError(failed.error).code).not.toBe(
|
||||
"cleanup_incomplete",
|
||||
);
|
||||
expect(projectAnalysisError(failed.error).code).toBe("execution_failure");
|
||||
expect(failed.error.cleanupPrimary).toBe(primary);
|
||||
expect(failed.error.diagnostics).toMatchObject({
|
||||
cleanup_failures: ["injected root cleanup failure"],
|
||||
cleanup_error: {
|
||||
details: {
|
||||
diagnostics: {
|
||||
cleanup_failures: ["injected root cleanup failure"],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
await decoder.close();
|
||||
@@ -108,6 +124,7 @@ it("settles every source-map cleanup failure when final close releases the exact
|
||||
prefix: "rea-source-map-final-cleanup-test-",
|
||||
});
|
||||
let attempts = 0;
|
||||
const launchCause = new Error("controlled launch failure");
|
||||
const decoder = new SourceMapDecoder({
|
||||
prepareOwnership: async () => {},
|
||||
createRuntime: async () => ({
|
||||
@@ -120,12 +137,16 @@ it("settles every source-map cleanup failure when final close releases the exact
|
||||
},
|
||||
}),
|
||||
launcher: async () => {
|
||||
throw new Error("controlled launch failure");
|
||||
throw launchCause;
|
||||
},
|
||||
});
|
||||
try {
|
||||
const initial = await decoder.trace(input);
|
||||
if (initial.ok) throw new Error("expected initial cleanup failure");
|
||||
expect(initial.error.cleanupPrimary?.cause).toBe(launchCause);
|
||||
expect((initial.error.cause as AggregateError).errors[0]).toBe(
|
||||
initial.error.cleanupPrimary,
|
||||
);
|
||||
const retry = await decoder.trace(input);
|
||||
if (retry.ok) throw new Error("expected retry cleanup failure");
|
||||
for (const failure of [initial.error, retry.error]) {
|
||||
@@ -139,11 +160,22 @@ it("settles every source-map cleanup failure when final close releases the exact
|
||||
for (const failure of [initial.error, retry.error]) {
|
||||
expect(failure.cleanupIncomplete).toBe(false);
|
||||
expect(failure.cleanupResources).toEqual([]);
|
||||
expect(projectAnalysisError(failure).code).not.toBe("cleanup_incomplete");
|
||||
expect(failure.diagnostics?.cleanup_failures).toEqual([
|
||||
expect.stringContaining("runtime cleanup attempt"),
|
||||
]);
|
||||
expect(projectAnalysisError(failure).code).toBe("execution_failure");
|
||||
}
|
||||
expect(initial.error.cleanupPrimary?.cause).toBe(launchCause);
|
||||
expect(initial.error.diagnostics).toMatchObject({
|
||||
cleanup_error: {
|
||||
details: {
|
||||
diagnostics: {
|
||||
cleanup_failures: ["runtime cleanup attempt 1 failed"],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(retry.error).toBeInstanceOf(SourceMapCleanupFailure);
|
||||
expect(retry.error.diagnostics?.cleanup_failures).toEqual([
|
||||
"runtime cleanup attempt 2 failed",
|
||||
]);
|
||||
} finally {
|
||||
await decoder.close();
|
||||
}
|
||||
|
||||
@@ -22,9 +22,9 @@ import {
|
||||
} from "./SourceMapCodecProtocol.js";
|
||||
import {
|
||||
SOURCE_MAP_CODEC_MEMORY_LIMIT_BYTES,
|
||||
SourceMapCleanupFailure,
|
||||
runSourceMapCommand,
|
||||
type SourceMapCleanupOwner,
|
||||
type SourceMapCleanupRetention,
|
||||
type SourceMapDecoderDependencies,
|
||||
} from "./SourceMapCommand.js";
|
||||
import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
|
||||
@@ -114,18 +114,18 @@ export class SourceMapDecoder implements WebSourceMapPort {
|
||||
new AnalysisCancelledError(OPERATION, { cause: options.signal.reason }),
|
||||
);
|
||||
const serialized = encoded.join("");
|
||||
const retainedCleanup: SourceMapCleanupRetention = {};
|
||||
const response = await runSourceMapCommand(
|
||||
serialized,
|
||||
input.path,
|
||||
options,
|
||||
this.dependencies,
|
||||
retainedCleanup,
|
||||
input.decodeTimeoutMs ?? DEFAULT_WEB_SOURCE_MAP_DECODE_TIMEOUT_MS,
|
||||
);
|
||||
if (!response.ok) {
|
||||
if (response.error instanceof SourceMapCleanupFailure)
|
||||
this.#pendingCleanup = response.error.cleanupOwner;
|
||||
return response;
|
||||
}
|
||||
if (retainedCleanup.owner !== undefined)
|
||||
this.#pendingCleanup = retainedCleanup.owner;
|
||||
if (!response.ok) return response;
|
||||
let value: unknown;
|
||||
try {
|
||||
value = JSON.parse(response.value);
|
||||
@@ -174,7 +174,7 @@ export class SourceMapDecoder implements WebSourceMapPort {
|
||||
async #retryCleanup(): Promise<AnalysisError | undefined> {
|
||||
const pending = this.#pendingCleanup;
|
||||
if (pending === undefined) return undefined;
|
||||
const failure = await pending.close(null);
|
||||
const failure = await pending.close();
|
||||
if (failure !== undefined) return failure;
|
||||
this.#pendingCleanup = undefined;
|
||||
return undefined;
|
||||
|
||||
@@ -2,7 +2,10 @@ import { expect, it, vi } from "vitest";
|
||||
import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js";
|
||||
import { ProviderAdapterError } from "../../../src/domain/providerAdapterError.js";
|
||||
import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js";
|
||||
import { runSourceMapCommand } from "../../../src/javascript/sourceMaps/SourceMapCommand.js";
|
||||
import {
|
||||
runSourceMapCommand,
|
||||
type SourceMapCleanupRetention,
|
||||
} from "../../../src/javascript/sourceMaps/SourceMapCommand.js";
|
||||
|
||||
it.each([
|
||||
null,
|
||||
@@ -11,6 +14,7 @@ it.each([
|
||||
"preserves opaque cancellation during ownership preparation: %s",
|
||||
async (reason) => {
|
||||
const controller = new AbortController();
|
||||
const retainedCleanup: SourceMapCleanupRetention = {};
|
||||
const createRuntime = vi.fn(async () => {
|
||||
throw new Error("Cancelled preparation must not acquire a runtime");
|
||||
});
|
||||
@@ -29,7 +33,9 @@ it.each([
|
||||
createRuntime,
|
||||
launcher,
|
||||
},
|
||||
retainedCleanup,
|
||||
);
|
||||
expect(retainedCleanup.owner).toBeUndefined();
|
||||
if (result.ok) throw new Error("Preparation must return cancellation");
|
||||
expect(result.error).toBeInstanceOf(AnalysisCancelledError);
|
||||
expect(result.error.cause).toBe(reason);
|
||||
@@ -46,19 +52,27 @@ it.each([
|
||||
|
||||
it("retains diagnostics for an unknown ownership preparation failure", async () => {
|
||||
const cause = new Error("unexpected ownership failure");
|
||||
const retainedCleanup: SourceMapCleanupRetention = {};
|
||||
const createRuntime = vi.fn(async () => {
|
||||
throw new Error("Failed preparation must not acquire a runtime");
|
||||
});
|
||||
const launcher = vi.fn(async () => {
|
||||
throw new Error("Failed preparation must not launch a codec");
|
||||
});
|
||||
const result = await runSourceMapCommand("{}", "/selected.map", undefined, {
|
||||
prepareOwnership: async () => {
|
||||
throw cause;
|
||||
const result = await runSourceMapCommand(
|
||||
"{}",
|
||||
"/selected.map",
|
||||
undefined,
|
||||
{
|
||||
prepareOwnership: async () => {
|
||||
throw cause;
|
||||
},
|
||||
createRuntime,
|
||||
launcher,
|
||||
},
|
||||
createRuntime,
|
||||
launcher,
|
||||
});
|
||||
retainedCleanup,
|
||||
);
|
||||
expect(retainedCleanup.owner).toBeUndefined();
|
||||
if (result.ok) throw new Error("Preparation must return its failure");
|
||||
expect(result.error).toBeInstanceOf(ProviderAdapterError);
|
||||
expect(result.error.cause).toBe(cause);
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { expect, it } from "vitest";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
import { runSourceMapCommand } from "../../../src/javascript/sourceMaps/SourceMapCommand.js";
|
||||
import {
|
||||
runSourceMapCommand,
|
||||
type SourceMapCleanupRetention,
|
||||
} from "../../../src/javascript/sourceMaps/SourceMapCommand.js";
|
||||
import { PrivateRuntimeRootUnavailableError } from "../../../src/process/PrivateRuntimeRootUnavailableError.js";
|
||||
|
||||
it.each([false, true])(
|
||||
@@ -8,16 +11,24 @@ it.each([false, true])(
|
||||
async (retryable) => {
|
||||
const reason = "Native runtime self-test failed with a sharing violation";
|
||||
let launches = 0;
|
||||
const result = await runSourceMapCommand("{}", "/selected.map", undefined, {
|
||||
prepareOwnership: async () => {},
|
||||
createRuntime: async () => {
|
||||
throw new PrivateRuntimeRootUnavailableError(reason, { retryable });
|
||||
const retainedCleanup: SourceMapCleanupRetention = {};
|
||||
const result = await runSourceMapCommand(
|
||||
"{}",
|
||||
"/selected.map",
|
||||
undefined,
|
||||
{
|
||||
prepareOwnership: async () => {},
|
||||
createRuntime: async () => {
|
||||
throw new PrivateRuntimeRootUnavailableError(reason, { retryable });
|
||||
},
|
||||
launcher: async () => {
|
||||
launches += 1;
|
||||
throw new Error("A refused runtime must not launch a codec");
|
||||
},
|
||||
},
|
||||
launcher: async () => {
|
||||
launches += 1;
|
||||
throw new Error("A refused runtime must not launch a codec");
|
||||
},
|
||||
});
|
||||
retainedCleanup,
|
||||
);
|
||||
expect(retainedCleanup.owner).toBeUndefined();
|
||||
expect(launches).toBe(0);
|
||||
if (result.ok) throw new Error("Unavailable runtime must refuse decoding");
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
|
||||
Reference in New Issue
Block a user