diff --git a/src/application/WebScriptExportService.ts b/src/application/WebScriptExportService.ts index 67beeb213c..7986f8bbbc 100644 --- a/src/application/WebScriptExportService.ts +++ b/src/application/WebScriptExportService.ts @@ -17,7 +17,7 @@ import { AnalysisInputError, AnalysisOutputError, } from "../domain/analysisErrorCore.js"; -import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { AnalysisError } from "../domain/analysisErrorBase.js"; import { createEvidence, type Evidence } from "../domain/evidence.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { analysisInputErrorFromIssues } from "../domain/inputIssueProjection.js"; @@ -36,12 +36,15 @@ import { WEB_SCRIPT_EXPORT_PROVIDER as PROVIDER } from "./InvestigationProviders import { readRegularFile, RegularFileCleanupFailure, + RegularFileReadFailure, retryRegularFileCleanup, } from "./RegularFileRead.js"; import { NonRegularFileReadError, RegularFileChangedError, + RegularFileOperationCancelled, } from "../filesystem/RegularFile.js"; +import { bindArtifactFileCleanup } from "../artifacts/ArtifactFileOperation.js"; import { regularFileCancellation } from "../filesystem/OwnedRegularFile.js"; const OPERATION = "export_web_scripts"; @@ -93,7 +96,10 @@ export const exportWebScriptsValidated = async ( ]), ); try { - options.signal?.throwIfAborted(); + if (options.signal?.aborted) + return err( + new AnalysisCancelledError(OPERATION, { cause: options.signal.reason }), + ); const prepared = await prepareCapture( input.capture_path, resources, @@ -101,7 +107,10 @@ export const exportWebScriptsValidated = async ( ); if (!prepared.ok) return prepared; const { bytes, loaded } = prepared.value; - options.signal?.throwIfAborted(); + if (options.signal?.aborted) + return err( + new AnalysisCancelledError(OPERATION, { cause: options.signal.reason }), + ); const sha256 = createHash("sha256").update(bytes).digest("hex"); const result = await publishWebScripts(input, loaded, sha256, { resources, @@ -137,7 +146,7 @@ export const exportWebScriptsValidated = async ( ), ); } catch (cause: unknown) { - return err(mapExportFailure(cause, input, options.signal)); + return err(mapExportFailure(cause, input)); } }; @@ -180,19 +189,24 @@ const prepareCapture = async ( const mapExportFailure = ( cause: unknown, input: ExportWebScriptsInput, - signal: AbortSignal | undefined, ): AnalysisError => { - if (cause instanceof WebScriptExportError) return cause; - if (cause instanceof AnalysisCancelledError && cause !== signal?.reason) - return cause; - if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) - return analysisErrorWithCleanupFailure( - new WebScriptExportError( - cause.reason, - input.output_directory, - cause.message, - { cause }, - ), + if (cause instanceof AnalysisError) return cause; + if (cause instanceof ArtifactReaderFailure) { + const primary = + cause.cancellation === undefined + ? new WebScriptExportError( + cause.reason, + input.output_directory, + cause.message, + { cause, partialObservation: cause.partialObservation }, + ) + : new AnalysisCancelledError(OPERATION, { + cause: cause.cancellation.reason, + partialObservation: cause.partialObservation, + }); + if (cause.cleanup === undefined) return primary; + const error = analysisErrorWithCleanupFailure( + primary, new ProviderCleanupError( PROVIDER.id, cause.cleanup.resources, @@ -201,20 +215,9 @@ const mapExportFailure = ( ), OPERATION, ); - if (signal?.aborted === true) - return new AnalysisCancelledError(OPERATION, { - cause: - cause instanceof ArtifactReaderFailure && cause.cause === signal.reason - ? signal.reason - : cause, - }); - if (cause instanceof ArtifactReaderFailure) - return new WebScriptExportError( - cause.reason, - input.output_directory, - cause.message, - { cause }, - ); + bindArtifactFileCleanup(cause, error); + return error; + } if (cause instanceof z.ZodError) return new AnalysisOutputError(OPERATION, cause.message, { cause }); if ( @@ -254,10 +257,14 @@ const readCapture = async ( signal: AbortSignal | undefined, ): Promise> => { try { - return ok({ bytes: await readRegularFile(path, { signal }) }); + return ok({ + bytes: await readRegularFile(path, { + signal, + cancellationFailure: (reason) => + new RegularFileOperationCancelled(reason), + }), + }); } catch (cause: unknown) { - if (signal?.aborted === true && cause === signal.reason) - return err(captureReadFailure(path, cause, signal)); if (cause instanceof RegularFileCleanupFailure) { const retry = await retryRegularFileCleanup(cause, resources, signal); const cleanup = { @@ -287,8 +294,7 @@ const readCapture = async ( } const primary = captureReadFailure( path, - retry.outcome.cause, - signal, + cause, retry.cleanup === undefined ? { cause: cause.cleanupCause } : undefined, ); return err( @@ -297,16 +303,29 @@ const readCapture = async ( : incompleteCaptureCleanup(primary, path, cleanup), ); } - return err(captureReadFailure(path, cause, signal)); + return err(captureReadFailure(path, cause)); } }; const captureReadFailure = ( path: string, - cause: unknown, - signal: AbortSignal | undefined, + failure: unknown, closeFailure?: { readonly cause: unknown }, ): AnalysisError => { + const cancellation = regularFileCancellation(failure); + if (cancellation !== undefined) + return new AnalysisCancelledError(OPERATION, { + cause: cancellation.reason, + }); + const primary = + failure instanceof RegularFileCleanupFailure && + failure.outcome.kind === "failed" + ? (failure.operationFailure ?? failure.outcome.cause) + : failure; + const cause = + primary instanceof RegularFileReadFailure + ? primary.operationCause + : primary; const causeWithClose = closeFailure === undefined ? cause @@ -315,8 +334,6 @@ const captureReadFailure = ( "Capture read and file-handle close both failed", { cause }, ); - if (signal?.aborted === true) - return new AnalysisCancelledError(OPERATION, { cause: causeWithClose }); const code = cause instanceof Error && "code" in cause ? String(cause.code) : ""; if (code === "EACCES" || code === "EPERM") diff --git a/src/browser/assets/PublishWebScripts.ts b/src/browser/assets/PublishWebScripts.ts index 0d083c010b..f264776812 100644 --- a/src/browser/assets/PublishWebScripts.ts +++ b/src/browser/assets/PublishWebScripts.ts @@ -46,6 +46,12 @@ export const publishWebScripts = async ( options: PublishWebScriptsOptions, ): Promise => options.resources.run(async () => { + const assertActive = (): void => { + if (options.signal?.aborted) + throw new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + }); + }; const planned = planWebScriptExport(capture.scripts); const scenarioResponseBodyCaptured = capture.kind === "browser-scenario" && @@ -67,10 +73,10 @@ export const publishWebScripts = async ( let result: WebScriptExportResult | undefined; try { tree = await SafeOutputTree.create(input.output_directory); - options.signal?.throwIfAborted(); + assertActive(); const records: ExportedWebScript[] = []; for (const [index, item] of planned.entries()) { - options.signal?.throwIfAborted(); + assertActive(); records.push( await publishPlannedScript(tree, item, index, options.signal), ); @@ -115,10 +121,19 @@ export const publishWebScripts = async ( { sha256: digest, bytes: bytes.byteLength }, options.signal, ); - options.signal?.throwIfAborted(); + assertActive(); await tree.commit(options.signal); return result; } catch (cause: unknown) { + const cancellation = + cause instanceof AnalysisCancelledError + ? cause + : cause instanceof ArtifactReaderFailure && + cause.cancellation !== undefined + ? new AnalysisCancelledError(OPERATION, { + cause: cause.cancellation.reason, + }) + : undefined; if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree; if (tree !== undefined) { const cleanup = await options.resources.release({ @@ -135,9 +150,9 @@ export const publishWebScripts = async ( tree.published && result !== undefined ? ({ kind: "web-script-export", result } as const) : undefined; - if (options.signal?.aborted === true) { + if (cancellation !== undefined) { const cancelled = new AnalysisCancelledError(OPERATION, { - cause: options.signal.reason, + cause: cancellation.cause, cleanup: { reason: diagnostic, resources: [tree.outputRoot] }, ...(partialObservation === undefined ? {} @@ -150,7 +165,7 @@ export const publishWebScripts = async ( cause instanceof ArtifactReaderFailure ? cause.reason : "io", tree.outputRoot, diagnostic, - { residualPaths: [tree.outputRoot] }, + { cause, residualPaths: [tree.outputRoot] }, ); if (partialObservation !== undefined) error.retainPartialObservation(partialObservation); @@ -160,7 +175,7 @@ export const publishWebScripts = async ( if (tree.published && result !== undefined) return result; } - throw cause; + throw cancellation ?? cause; } }); diff --git a/tests/boundary/filesystem/webScriptExport.test.ts b/tests/boundary/filesystem/webScriptExport.test.ts index 975aafc9e4..ecb173287c 100644 --- a/tests/boundary/filesystem/webScriptExport.test.ts +++ b/tests/boundary/filesystem/webScriptExport.test.ts @@ -513,6 +513,76 @@ describe("captured script publication failures and cleanup", () => { }); }); +describe("independent script publication failure", () => { + it.each( + [false, true].flatMap((rollbackFails) => + [false, true].map((sameReason) => ({ rollbackFails, sameReason })), + ), + )( + "keeps write failure primary after cancellation (rollback fails: $rollbackFails, same reason: $sameReason)", + async ({ rollbackFails, sameReason }) => { + const { input, resources } = await setup(); + const controller = new AbortController(); + const physical = Object.assign( + new Error("physical output write failed"), + { code: "EIO" }, + ); + openMock.mockImplementation(async (...args) => { + const handle = await actualFs.open(...args); + if ( + String(args[0]).startsWith(input.output_directory) && + typeof args[1] === "number" && + (args[1] & constants.O_WRONLY) !== 0 + ) + vi.spyOn(handle, "write").mockImplementation(async () => { + controller.abort( + sameReason ? physical : { request: "later cancellation" }, + ); + throw physical; + }); + return handle; + }); + const rollback = rollbackFails + ? vi + .spyOn(SafeOutputTree.prototype, "rollback") + .mockRejectedValue(new Error("rollback refused")) + : undefined; + let result: Awaited>; + try { + result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected physical output failure"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: rollbackFails + ? "cleanup_incomplete" + : "artifact_operation_failed", + details: { reason: "io" }, + }); + expect(result.error.partialObservation).toBeUndefined(); + if (rollbackFails) + expect(result.error.cleanupResources).toEqual([ + input.output_directory, + ]); + else { + expect(result.error.cause).toBe(physical); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + } + } finally { + rollback?.mockRestore(); + } + await resources.close(); + if (result.ok) throw new Error("Expected failure"); + expect(result.error.cleanupIncomplete).toBe(false); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + }, + ); +}); + describe("captured script cancellation finalization", () => { it("keeps the caller reason when rollback also fails after an interrupted write", async () => { const { input, resources } = await setup( @@ -530,9 +600,10 @@ describe("captured script cancellation finalization", () => { .spyOn(SafeOutputTree, "create") .mockImplementationOnce(async (...args) => { const tree = await create(...args); - vi.spyOn(tree, "write").mockImplementationOnce(async () => { + const write = tree.write.bind(tree); + vi.spyOn(tree, "write").mockImplementationOnce(async (...args) => { controller.abort(reason); - throw new Error("injected write failure"); + return write(...args); }); vi.spyOn(tree, "rollback").mockRejectedValueOnce( new Error("injected rollback failure"), @@ -887,6 +958,78 @@ describe("captured script path and permission failures", () => { ); }); +describe("capture read failure provenance", () => { + it.each( + ["EACCES", "EIO"].flatMap((code) => + [0, 1, 2].flatMap((failedCloses) => + [false, true].map((sameReason) => ({ code, failedCloses, sameReason })), + ), + ), + )( + "keeps $code primary through cancellation and $failedCloses failed closes (same reason: $sameReason)", + async ({ code, failedCloses, sameReason }) => { + const { input, resources } = await setup(); + const controller = new AbortController(); + const physical = Object.assign( + new Error("physical capture read failed"), + { code }, + ); + const closeFailure = new Error("capture close failed"); + let handle: Awaited> | undefined; + let closeCalls = 0; + openMock.mockImplementation(async (...args) => { + const opened = await actualFs.open(...args); + if (String(args[0]) !== input.capture_path) return opened; + handle = opened; + vi.spyOn(opened, "read").mockImplementation(async () => { + controller.abort( + sameReason ? physical : { request: "later cancellation" }, + ); + throw physical; + }); + const close = opened.close.bind(opened); + vi.spyOn(opened, "close").mockImplementation(async () => { + closeCalls++; + if (closeCalls <= failedCloses) throw closeFailure; + await close(); + }); + return opened; + }); + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected physical capture read failure"); + const expectedCode = + code === "EACCES" ? "access_denied" : "artifact_operation_failed"; + if (failedCloses === 2) { + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [input.capture_path], + diagnostics: { primary_error: { code: expectedCode } }, + }, + }); + expect(handle?.fd).toBeGreaterThanOrEqual(0); + } else expect(projectAnalysisError(result.error).code).toBe(expectedCode); + const primary = result.error.cleanupPrimary ?? result.error; + if (failedCloses === 1) + expect(primary.cause).toMatchObject({ + cause: physical, + errors: [physical, closeFailure], + }); + else expect(primary.cause).toBe(physical); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + await resources.close(); + expect(handle?.fd).toBe(-1); + expect(closeCalls).toBe(failedCloses + 1); + expect(result.error.cleanupIncomplete).toBe(false); + expect(projectAnalysisError(result.error).code).toBe(expectedCode); + }, + ); +}); + describe("capture read and close failure", () => { it("retains a handle after capture read and close both fail", async () => { const { input, resources } = await setup();