Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 51 additions & 2 deletions extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@
*/

import { assertEquals, assertExists, assertRejects, assertStringIncludes } from "@std/assert";
import { describe, it } from "@std/testing/bdd";
import { afterEach, describe, it } from "@std/testing/bdd";
Comment thread
kojiwakayama marked this conversation as resolved.
Outdated
import { createRequire } from "node:module";

import { EsbuildBundler, isLiveEsbuildServiceProcess } from "./esbuild-bundler.ts";
import {
__recordOwnershipErrorForTests,
__resetOwnershipErrorForTests,
EsbuildBundler,
isLiveEsbuildServiceProcess,
} from "./esbuild-bundler.ts";
import { rebuildContextWithSignal } from "./context-build-lifecycle.ts";

const childProcess = createRequire(import.meta.url)("node:child_process") as {
Expand Down Expand Up @@ -811,6 +816,50 @@ describe("EsbuildBundler.bundle", () => {
});
});

describe("ownership error cause", () => {
afterEach(() => {
__resetOwnershipErrorForTests();
});

it("adopts the underlying failure when the latch was set before it surfaced", () => {
// The latch is created before the operation settles, so it starts without a
// cause. Discarding the cause that arrives afterwards is what made every
// real esbuild failure surface as a lifecycle problem instead.
__resetOwnershipErrorForTests();
__recordOwnershipErrorForTests();
const error = __recordOwnershipErrorForTests(new Error("spawn ENOENT esbuild"));

assertStringIncludes(error.message, "module-wide adapter");
assertStringIncludes(error.message, "spawn ENOENT esbuild");
assertEquals((error.cause as Error).message, "spawn ENOENT esbuild");
});

it("reports a cause supplied on the first record", () => {
__resetOwnershipErrorForTests();
const error = __recordOwnershipErrorForTests(new Error("binary missing"));

assertStringIncludes(error.message, "binary missing");
assertEquals((error.cause as Error).message, "binary missing");
});

it("keeps the first cause rather than overwriting it", () => {
__resetOwnershipErrorForTests();
__recordOwnershipErrorForTests(new Error("first failure"));
const error = __recordOwnershipErrorForTests(new Error("second failure"));

assertEquals((error.cause as Error).message, "first failure");
assertStringIncludes(error.message, "first failure");
});

it("stays usable when there is no underlying failure", () => {
__resetOwnershipErrorForTests();
const error = __recordOwnershipErrorForTests();

assertStringIncludes(error.message, "module-wide adapter");
assertEquals(error.cause, undefined);
});
});

describe("EsbuildBundler unsupported lifecycle ownership", () => {
it("rejects shutdown after a raw service generation replaces the managed one", async () => {
const observation = observeEsbuildServices();
Expand Down
53 changes: 47 additions & 6 deletions extensions/ext-bundler-esbuild/src/esbuild-bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,40 @@ let activeOperationsIdle: Promise<void> = Promise.resolve();
let resolveActiveOperationsIdle: (() => void) | null = null;
const operationScopes = new AsyncLocalStorage<OperationScope>();

const OWNERSHIP_ERROR_MESSAGE =
"[ext-bundler-esbuild] Cannot own an esbuild service started outside the module-wide adapter; restart the process and use only the Bundler contract";

function describeCause(cause: unknown): string {
if (cause === undefined) return "";
const detail = cause instanceof Error ? cause.message : String(cause);
return detail ? ` (underlying failure: ${detail})` : "";
Comment thread
kojiwakayama marked this conversation as resolved.
}

/**
* Latch the ownership failure, keeping whatever really went wrong.
*
* The latch is permanent, so the first call decides the error every later
* operation sees. That call is often made before the operation settles, when no
* cause is known yet; adopting the cause afterwards is what keeps the real
* failure visible instead of reporting a lifecycle problem that may not be the
* actual one. The cause is folded into the message because callers log
* `error.message` and would otherwise never print the chain.
*/
function recordOwnershipError(cause?: unknown): Error {
const message =
"[ext-bundler-esbuild] Cannot own an esbuild service started outside the module-wide adapter; restart the process and use only the Bundler contract";
esbuildOwnershipError ??= new Error(message, cause === undefined ? undefined : { cause });
return esbuildOwnershipError;
const existing = esbuildOwnershipError;
if (!existing) {
esbuildOwnershipError = new Error(
`${OWNERSHIP_ERROR_MESSAGE}${describeCause(cause)}`,
cause === undefined ? undefined : { cause },
);
return esbuildOwnershipError;
}

if (cause !== undefined && existing.cause === undefined) {
existing.cause = cause;
existing.message = `${OWNERSHIP_ERROR_MESSAGE}${describeCause(cause)}`;
}
return existing;
}

async function getEsbuild(): Promise<EsbuildModule> {
Expand Down Expand Up @@ -220,6 +249,16 @@ function isEsbuildServiceSpawn(spawnArgs: unknown[]): boolean {
* Native Node represents an active child with `null` exit fields. Some
* compatible runtimes leave those fields undefined until the child exits.
*/
/** The ownership latch is module-wide; tests must clear it between cases. */
export function __resetOwnershipErrorForTests(): void {
esbuildOwnershipError = null;
}

/** Exercise the latch without starting a real esbuild service. */
export function __recordOwnershipErrorForTests(cause?: unknown): Error {
return recordOwnershipError(cause);
}

export function isLiveEsbuildServiceProcess(
child: Pick<ChildProcess, "killed" | "exitCode" | "signalCode">,
): boolean {
Expand Down Expand Up @@ -270,10 +309,12 @@ function invokeEsbuild<T extends Promise<unknown>>(operation: () => T): T {

const ownedService = capturedService ?? esbuildService;
if (!ownedService || !isLiveService(ownedService)) {
const ownershipError = recordOwnershipError();
// Latch only once the operation settles. Recording up front would fix a
// causeless error in place and the rejection below could no longer attach
// what actually failed.
return result.then(
() => {
throw ownershipError;
throw recordOwnershipError();
Comment thread
kojiwakayama marked this conversation as resolved.
},
(cause) => {
throw recordOwnershipError(cause);
Expand Down