mirror of
https://github.com/morluto/rea.git
synced 2026-11-12 09:52:27 +00:00
fix(setup): retain Hopper operation failure attribution (#9958)
* fix(setup): retain Hopper operation failure attribution * fix(setup): retain falsy Hopper abort reasons from body reads After response headers, Node's fetch body reader rejects a null, false, 0, or empty-string abort with a fresh AbortError instead of that reason. Replace that substituted error with the caller's reason. A body AbortError while the caller has not aborted still propagates. * fix(setup): preserve independent response body aborts * fix(setup): identify aborts through owned transport --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This commit is contained in:
1 parent
83f0a4d238
commit
67f74c5fb9
8 files changed
+414
-22
No files matched your search
@@ -0,0 +1,180 @@
|
||||
import { createServer } from "node:http";
|
||||
import { once } from "node:events";
|
||||
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
|
||||
import { downloadHopperPackage } from "./HopperDownload.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it.each(["physical", "caller", "node-wrapper", "default-abort"] as const)(
|
||||
"attributes fetch rejection to the operation that failed (%s)",
|
||||
async (kind) => {
|
||||
const controller = new AbortController();
|
||||
const reason = { caller: "stop download" };
|
||||
const physical = new TypeError("network read failed");
|
||||
const wrapped = Object.assign(
|
||||
new Error("operation aborted", { cause: reason }),
|
||||
{
|
||||
name: "AbortError",
|
||||
code: "ABORT_ERR",
|
||||
},
|
||||
);
|
||||
let thrown: unknown;
|
||||
vi.stubGlobal("fetch", async () => {
|
||||
if (kind === "default-abort") controller.abort();
|
||||
else controller.abort(reason);
|
||||
thrown =
|
||||
kind === "physical"
|
||||
? physical
|
||||
: kind === "node-wrapper"
|
||||
? wrapped
|
||||
: controller.signal.reason;
|
||||
throw thrown;
|
||||
});
|
||||
const download = downloadHopperPackage("https://fixture.invalid/archive", {
|
||||
signal: controller.signal,
|
||||
});
|
||||
if (kind === "physical")
|
||||
expect(await download).toMatchObject({
|
||||
ok: false,
|
||||
error: { kind: "network", detail: physical.message },
|
||||
});
|
||||
else await expect(download).rejects.toBe(thrown);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([false, true])(
|
||||
"attributes an expired download deadline to its own rejection (%s)",
|
||||
async (interrupted) => {
|
||||
vi.useFakeTimers();
|
||||
vi.stubGlobal("fetch", async (_url: string, options: RequestInit) => {
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 2));
|
||||
throw interrupted
|
||||
? options.signal?.reason
|
||||
: new TypeError("physical network failure");
|
||||
});
|
||||
const download = downloadHopperPackage("https://fixture.invalid/archive", {
|
||||
timeoutMs: 1,
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(2);
|
||||
expect(await download).toMatchObject({
|
||||
ok: false,
|
||||
error: interrupted
|
||||
? { kind: "timeout", limit: "deadline", timeoutMs: 1 }
|
||||
: { kind: "network", detail: "physical network failure" },
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each([null, false, 0, ""] as const)(
|
||||
"retains a falsy caller abort reason when the response body is cancelled (%j)",
|
||||
async (reason) => {
|
||||
const controller = new AbortController();
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
async (_url: string, options: RequestInit) =>
|
||||
new Response(
|
||||
new ReadableStream({
|
||||
pull(stream) {
|
||||
controller.abort(reason);
|
||||
stream.error(options.signal?.reason);
|
||||
},
|
||||
}),
|
||||
{ status: 200 },
|
||||
),
|
||||
);
|
||||
await expect(
|
||||
downloadHopperPackage("https://fixture.invalid/archive", {
|
||||
signal: controller.signal,
|
||||
}),
|
||||
).rejects.toBe(reason);
|
||||
},
|
||||
);
|
||||
|
||||
it("does not treat an AbortError body failure as cancellation when the caller did not abort", async () => {
|
||||
const failure = new DOMException("The operation was aborted", "AbortError");
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
async () =>
|
||||
new Response(
|
||||
new ReadableStream({
|
||||
pull(stream) {
|
||||
stream.error(failure);
|
||||
},
|
||||
}),
|
||||
{ status: 200 },
|
||||
),
|
||||
);
|
||||
await expect(
|
||||
downloadHopperPackage("https://fixture.invalid/archive"),
|
||||
).rejects.toMatchObject({ name: "AbortError", code: DOMException.ABORT_ERR });
|
||||
});
|
||||
|
||||
it.each([null, false, 0, "", { caller: "stop download" }] as const)(
|
||||
"preserves an independent body failure when the caller also aborts (%j)",
|
||||
async (reason) => {
|
||||
const controller = new AbortController();
|
||||
const failure = new DOMException(
|
||||
"independent response failure",
|
||||
"AbortError",
|
||||
);
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
async () =>
|
||||
new Response(
|
||||
new ReadableStream<Uint8Array>({
|
||||
pull(stream) {
|
||||
controller.abort(reason);
|
||||
stream.error(failure);
|
||||
},
|
||||
}),
|
||||
),
|
||||
);
|
||||
await expect(
|
||||
downloadHopperPackage("https://fixture.invalid/archive", {
|
||||
signal: controller.signal,
|
||||
}),
|
||||
).rejects.toBe(failure);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([null, false, { caller: "cancel" }] as const)(
|
||||
"retains the exact caller reason through a native response body read (%j)",
|
||||
async (reason) => {
|
||||
const server = createServer((_request, response) => {
|
||||
response.writeHead(200);
|
||||
response.write("first chunk");
|
||||
});
|
||||
server.listen(0, "127.0.0.1");
|
||||
await once(server, "listening");
|
||||
const address = server.address();
|
||||
if (address === null || typeof address === "string")
|
||||
throw new Error("Expected a TCP listener");
|
||||
const controller = new AbortController();
|
||||
const nativeFetch = globalThis.fetch;
|
||||
vi.stubGlobal("fetch", async (_url: string, options: RequestInit) => {
|
||||
const response = await nativeFetch(
|
||||
`http://127.0.0.1:${address.port}/`,
|
||||
options,
|
||||
);
|
||||
setImmediate(() => controller.abort(reason));
|
||||
return response;
|
||||
});
|
||||
try {
|
||||
await expect(
|
||||
downloadHopperPackage("https://fixture.invalid/archive", {
|
||||
signal: controller.signal,
|
||||
}),
|
||||
).rejects.toBe(reason);
|
||||
} finally {
|
||||
server.closeAllConnections();
|
||||
await new Promise<void>((resolve, reject) =>
|
||||
server.close((error) => (error ? reject(error) : resolve())),
|
||||
);
|
||||
}
|
||||
},
|
||||
);
|
||||
@@ -60,6 +60,19 @@ export const HOPPER_DOWNLOAD_TIMEOUT_MS = 15 * 60_000;
|
||||
/** Small release feed and key requests must not use the package bounds. */
|
||||
export const HOPPER_METADATA_TIMEOUT_MS = 10_000;
|
||||
|
||||
/** Attribute a raw rejection to this caller's exact cancellation. */
|
||||
export const isHopperRequestCancellation = (
|
||||
cause: unknown,
|
||||
signal: AbortSignal | undefined,
|
||||
): boolean =>
|
||||
signal?.aborted === true &&
|
||||
(cause === signal.reason ||
|
||||
(cause instanceof Error &&
|
||||
cause.name === "AbortError" &&
|
||||
"code" in cause &&
|
||||
cause.code === "ABORT_ERR" &&
|
||||
cause.cause === signal.reason));
|
||||
|
||||
/**
|
||||
* Bound observed response bytes before retaining them, including chunked bodies.
|
||||
* Expected transport outcomes are returned; caller cancellation still rejects
|
||||
@@ -115,19 +128,34 @@ export const downloadHopperPackage = async (
|
||||
expire("stalled", HOPPER_DOWNLOAD_STALL_MS);
|
||||
}, HOPPER_DOWNLOAD_STALL_MS);
|
||||
};
|
||||
const selectedSignal =
|
||||
options.signal === undefined
|
||||
? deadline.signal
|
||||
: AbortSignal.any([options.signal, deadline.signal]);
|
||||
// Node may replace falsy abort reasons during body reads. Send a unique,
|
||||
// truthy reason through fetch so only its own rejection can be attributed.
|
||||
const transport = new AbortController();
|
||||
const transportReason = {};
|
||||
const cancelTransport = () => transport.abort(transportReason);
|
||||
if (selectedSignal.aborted) cancelTransport();
|
||||
else
|
||||
selectedSignal.addEventListener("abort", cancelTransport, { once: true });
|
||||
try {
|
||||
progressed(0);
|
||||
return await readHopperResponse(url, reported, {
|
||||
signal:
|
||||
options.signal === undefined
|
||||
? deadline.signal
|
||||
: AbortSignal.any([options.signal, deadline.signal]),
|
||||
signal: transport.signal,
|
||||
maxBytes,
|
||||
progressed,
|
||||
});
|
||||
} catch (cause: unknown) {
|
||||
if (options.signal?.aborted === true) throw cause;
|
||||
if (expired !== undefined)
|
||||
} catch (observed: unknown) {
|
||||
const cause = isHopperRequestCancellation(observed, transport.signal)
|
||||
? selectedSignal.reason
|
||||
: observed;
|
||||
if (isHopperRequestCancellation(cause, options.signal)) throw cause;
|
||||
if (
|
||||
expired !== undefined &&
|
||||
isHopperRequestCancellation(cause, deadline.signal)
|
||||
)
|
||||
return err({ url: reported, kind: "timeout", ...expired });
|
||||
// Fetch reports network errors (DNS, TLS, reset, truncated body) as TypeError.
|
||||
if (cause instanceof TypeError)
|
||||
@@ -138,6 +166,7 @@ export const downloadHopperPackage = async (
|
||||
});
|
||||
throw cause;
|
||||
} finally {
|
||||
selectedSignal.removeEventListener("abort", cancelTransport);
|
||||
clearTimeout(overall);
|
||||
clearTimeout(stall);
|
||||
}
|
||||
|
||||
@@ -646,3 +646,69 @@ it("reports missing ldd as unresolved launcher inspection while retaining actual
|
||||
await host.close?.();
|
||||
}
|
||||
});
|
||||
|
||||
it.each(["physical", "abort-name", "caller", "node-wrapper"] as const)(
|
||||
"attributes a rejected archive write through cleanup (%s)",
|
||||
async (kind) => {
|
||||
const host = new RecordingLinuxHost();
|
||||
const controller = new AbortController();
|
||||
const reason = { caller: "stop during archive write" };
|
||||
const physical = Object.assign(new Error("archive write denied"), {
|
||||
code: "EACCES",
|
||||
name: kind === "abort-name" ? "AbortError" : "Error",
|
||||
});
|
||||
const wrapped = Object.assign(
|
||||
new Error("operation aborted", { cause: reason }),
|
||||
{
|
||||
name: "AbortError",
|
||||
code: "ABORT_ERR",
|
||||
},
|
||||
);
|
||||
host.writeArchive = async () => {
|
||||
controller.abort(reason);
|
||||
throw kind === "caller"
|
||||
? reason
|
||||
: kind === "node-wrapper"
|
||||
? wrapped
|
||||
: physical;
|
||||
};
|
||||
host.cleanupFails = true;
|
||||
const result = await installLinuxHopper(host, {
|
||||
signal: controller.signal,
|
||||
});
|
||||
const cancelled = kind === "caller" || kind === "node-wrapper";
|
||||
expect(result).toMatchObject({
|
||||
status: "failed",
|
||||
reason: cancelled ? "cancelled" : "archive_write",
|
||||
cleanupFailure: {
|
||||
path: "/tmp/rea test",
|
||||
reason: "temporary path is busy",
|
||||
},
|
||||
});
|
||||
if (result.status !== "failed") throw new Error("Expected failed write");
|
||||
if (cancelled) expect(result.cancellationReason).toBe(reason);
|
||||
else
|
||||
expect(result.operationFailure).toMatchObject({
|
||||
operation: "archive_write",
|
||||
systemCode: "EACCES",
|
||||
detail: physical.message,
|
||||
});
|
||||
expect(host.installed).toEqual([]);
|
||||
expect(host.cleaned).toEqual(["/tmp/rea test"]);
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves the package producer's cancellation when a later caller reason differs", async () => {
|
||||
const host = new RecordingLinuxHost();
|
||||
const controller = new AbortController();
|
||||
const producerReason = { producer: "package operation cancelled" };
|
||||
host.installPackage = async () => {
|
||||
controller.abort({ caller: "later stop" });
|
||||
return { status: "cancelled", cause: producerReason };
|
||||
};
|
||||
const result = await installLinuxHopper(host, { signal: controller.signal });
|
||||
expect(result).toMatchObject({ status: "failed", reason: "cancelled" });
|
||||
if (result.status !== "failed") throw new Error("Expected cancellation");
|
||||
expect(result.cancellationReason).toBe(producerReason);
|
||||
expect(host.cleaned).toEqual(["/tmp/rea test"]);
|
||||
});
|
||||
@@ -10,6 +10,7 @@ import { execFileOutput } from "../process/ExecFileOutput.js";
|
||||
import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
|
||||
import {
|
||||
downloadHopperPackage,
|
||||
isHopperRequestCancellation,
|
||||
type HopperDownload,
|
||||
type HopperRequestFailure,
|
||||
} from "./HopperDownload.js";
|
||||
@@ -486,19 +487,14 @@ export const installLinuxHopper = async (
|
||||
return { status: "installed", launcherPath };
|
||||
})();
|
||||
} catch (cause: unknown) {
|
||||
const cancelled = isHopperRequestCancellation(cause, options.signal);
|
||||
result = {
|
||||
status: "failed",
|
||||
reason:
|
||||
isAborted(options.signal) || isAbortCause(cause) ? "cancelled" : stage,
|
||||
reason: cancelled ? "cancelled" : stage,
|
||||
operationFailure: operationFailure(stage, cause, operationPath),
|
||||
...(cancelled ? { cancellationReason: options.signal?.reason } : {}),
|
||||
};
|
||||
}
|
||||
if (
|
||||
result.status === "failed" &&
|
||||
result.reason === "cancelled" &&
|
||||
isAborted(options.signal)
|
||||
)
|
||||
result = { ...result, cancellationReason: options.signal?.reason };
|
||||
if (temporary === undefined) return result;
|
||||
try {
|
||||
await host.cleanup(temporary);
|
||||
@@ -739,6 +735,7 @@ const publisherFailure = (
|
||||
status: "failed",
|
||||
reason,
|
||||
verificationFailure: error,
|
||||
...(reason === "cancelled" ? { cancellationReason: error.cause } : {}),
|
||||
...(error.download === undefined ? {} : { download: error.download }),
|
||||
});
|
||||
|
||||
@@ -868,5 +865,3 @@ const versionAtLeast = (
|
||||
): boolean =>
|
||||
version !== undefined &&
|
||||
Number.parseInt(version.split(".")[0] ?? "0", 10) >= minimum;
|
||||
const isAbortCause = (cause: unknown): boolean =>
|
||||
cause instanceof Error && cause.name === "AbortError";
|
||||
@@ -0,0 +1,50 @@
|
||||
import { writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { expect, it } from "vitest";
|
||||
|
||||
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
|
||||
import { ExecFileOutputCancelledError } from "../process/ExecFileOutput.js";
|
||||
import { LinuxHopperPublisher } from "./LinuxHopperPublisher.js";
|
||||
|
||||
it.each(["physical", "caller", "node-wrapper", "producer"] as const)(
|
||||
"attributes package-family observation failure after hashing (%s)",
|
||||
async (kind) => {
|
||||
const root = await createTestTempDirectory("rea-publisher-cancellation-");
|
||||
const path = join(root, "inert-launcher");
|
||||
await writeFile(path, "inert launcher bytes");
|
||||
const controller = new AbortController();
|
||||
const reason = { caller: "stop inspection" };
|
||||
const physical = new Error("distribution inspection failed");
|
||||
const wrapped = Object.assign(
|
||||
new Error("operation aborted", { cause: reason }),
|
||||
{
|
||||
name: "AbortError",
|
||||
code: "ABORT_ERR",
|
||||
},
|
||||
);
|
||||
const publisher = new LinuxHopperPublisher({ HOME: root }, async () => {
|
||||
if (kind !== "producer") controller.abort(reason);
|
||||
throw kind === "physical"
|
||||
? physical
|
||||
: kind === "caller"
|
||||
? reason
|
||||
: kind === "node-wrapper"
|
||||
? wrapped
|
||||
: new ExecFileOutputCancelledError(reason);
|
||||
});
|
||||
try {
|
||||
const result = await publisher.inspectLauncher(path, controller.signal);
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
reason:
|
||||
kind === "physical" ? "verification_unavailable" : "cancelled",
|
||||
},
|
||||
});
|
||||
if (result.ok) throw new Error("Expected failed observation");
|
||||
expect(result.error.cause).toBe(kind === "physical" ? physical : reason);
|
||||
} finally {
|
||||
await publisher.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
@@ -27,13 +27,17 @@ import {
|
||||
} from "../filesystem/OwnedRegularFile.js";
|
||||
import { readBoundedFileBytes } from "../process/BoundedFileBytes.js";
|
||||
import { verifyDetachedPgpSignature } from "../process/DetachedPgpSignature.js";
|
||||
import { execFileOutput } from "../process/ExecFileOutput.js";
|
||||
import {
|
||||
execFileOutput,
|
||||
ExecFileOutputCancelledError,
|
||||
} from "../process/ExecFileOutput.js";
|
||||
import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js";
|
||||
import { waitForAbortableDelay } from "../process/ProviderDeadline.js";
|
||||
import { acquireHopperReleaseCacheLease } from "../process/HopperLease.js";
|
||||
import type { OwnedLease } from "../process/OwnedLease.js";
|
||||
import {
|
||||
downloadHopperPackage,
|
||||
isHopperRequestCancellation,
|
||||
HOPPER_DOWNLOAD_BYTES,
|
||||
HOPPER_METADATA_TIMEOUT_MS,
|
||||
} from "./HopperDownload.js";
|
||||
@@ -232,7 +236,12 @@ export class LinuxHopperPublisher {
|
||||
? "cancelled"
|
||||
: "publisher_verification_failed",
|
||||
verified.error.message,
|
||||
{ cause: verified.error },
|
||||
{
|
||||
cause:
|
||||
verified.error.reason === "cancelled"
|
||||
? verified.error.cause
|
||||
: verified.error,
|
||||
},
|
||||
),
|
||||
);
|
||||
const mainExeHash = await this.#withTemporary(
|
||||
@@ -1079,6 +1088,12 @@ const releaseFailure = (
|
||||
signal: AbortSignal | undefined,
|
||||
fallback: HopperReleaseError["reason"],
|
||||
): HopperReleaseError => {
|
||||
if (isHopperRequestCancellation(cause, signal))
|
||||
return new HopperReleaseError(
|
||||
"cancelled",
|
||||
"Hopper publisher verification was cancelled.",
|
||||
{ cause: signal?.reason },
|
||||
);
|
||||
if (cause instanceof HopperReleaseError) return cause;
|
||||
if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined)
|
||||
return new HopperReleaseError(
|
||||
@@ -1093,11 +1108,11 @@ const releaseFailure = (
|
||||
"Hopper publisher verification was cancelled.",
|
||||
{ cause: cancellation.reason },
|
||||
);
|
||||
if (!(cause instanceof StableFileHashFailure) && signal?.aborted === true)
|
||||
if (cause instanceof ExecFileOutputCancelledError)
|
||||
return new HopperReleaseError(
|
||||
"cancelled",
|
||||
"Hopper publisher verification was cancelled.",
|
||||
{ cause },
|
||||
{ cause: cause.cause },
|
||||
);
|
||||
return new HopperReleaseError(
|
||||
fallback,
|
||||
|
||||
@@ -613,3 +613,57 @@ it("retains the completed installation but skips opening when cancelled during i
|
||||
expect(host.unmounts).toBe(1);
|
||||
expect(host.cleanups).toBe(1);
|
||||
});
|
||||
|
||||
it.each(["physical", "abort-name", "caller", "node-wrapper"] as const)(
|
||||
"attributes a rejected package write and retains cleanup (%s)",
|
||||
async (kind) => {
|
||||
const host = new FakeMacHopperHost();
|
||||
const controller = new AbortController();
|
||||
const reason = { caller: "stop during package write" };
|
||||
const physical = Object.assign(new Error("package write denied"), {
|
||||
name: kind === "abort-name" ? "AbortError" : "Error",
|
||||
});
|
||||
const wrapped = Object.assign(
|
||||
new Error("operation aborted", { cause: reason }),
|
||||
{
|
||||
name: "AbortError",
|
||||
code: "ABORT_ERR",
|
||||
},
|
||||
);
|
||||
host.writePackage = async () => {
|
||||
controller.abort(reason);
|
||||
throw kind === "caller"
|
||||
? reason
|
||||
: kind === "node-wrapper"
|
||||
? wrapped
|
||||
: physical;
|
||||
};
|
||||
let retained: (() => Promise<void>) | undefined;
|
||||
let release = false;
|
||||
host.cleanup = async () => {
|
||||
if (!release) throw new Error("temporary root busy");
|
||||
};
|
||||
const result = await installMacHopper(
|
||||
{ signal: controller.signal },
|
||||
{
|
||||
...host,
|
||||
retainCleanup: (cleanup, resources) => {
|
||||
expect(resources).toEqual(["/tmp/rea-hopper-test"]);
|
||||
retained = cleanup;
|
||||
},
|
||||
},
|
||||
);
|
||||
expect(result).toMatchObject({
|
||||
status: "failed",
|
||||
reason:
|
||||
kind === "caller" || kind === "node-wrapper" ? "cancelled" : "mount",
|
||||
cleanupFailure: expect.stringContaining("temporary root busy"),
|
||||
});
|
||||
if (kind === "physical" || kind === "abort-name")
|
||||
expect(result).toMatchObject({ diagnostic: physical.message });
|
||||
expect(host.installs).toBe(0);
|
||||
expect(retained).toBeTypeOf("function");
|
||||
release = true;
|
||||
await retained?.();
|
||||
},
|
||||
);
|
||||
@@ -22,6 +22,7 @@ import { macDeveloperIdRequirement } from "../process/MacCodeSignature.js";
|
||||
import { macHopperUserApplicationPath } from "./MacHopperPaths.js";
|
||||
import {
|
||||
downloadHopperPackage,
|
||||
isHopperRequestCancellation,
|
||||
HOPPER_METADATA_TIMEOUT_MS,
|
||||
type HopperDownload,
|
||||
type HopperRequestFailure,
|
||||
@@ -310,7 +311,9 @@ export const installMacHopper = async (
|
||||
} catch (cause: unknown) {
|
||||
result = {
|
||||
status: "failed",
|
||||
reason: options.signal?.aborted === true ? "cancelled" : installStage,
|
||||
reason: isHopperRequestCancellation(cause, options.signal)
|
||||
? "cancelled"
|
||||
: installStage,
|
||||
diagnostic: failureMessage(cause),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user