fix(core,cli): register tasks loaded via dynamic import during run (#3688)
On a warm worker process, a task whose `task()` definition is loaded via `await import(...)` from inside another task's `run()` could end up permanently missing from the catalog: the `task()` call fired with no `_currentFileContext` set, `registerTaskMetadata` silently returned, and Node's ESM module cache then blocked the worker's setContext + re-import recovery from ever firing the call again. Subsequent runs of that task on the same warm process failed with `COULD_NOT_FIND_EXECUTOR` until the process hit `maxExecutionsPerProcess` and exited. All five of these had to coincide on the same worker for the bug to surface: 1. `processKeepAlive` enabled (so catalog state survives across runs). 2. A `run()` function (or lifecycle hook) does `await import(...)`. 3. The import's transitive static graph reaches a `task()` / `schemaTask()` call. 4. The task containing the dynamic import is the **first** task to run on a given warm worker process — so the dropped `task()` calls fire on this process for the first time, are silently dropped, and Node's module cache locks the wrong outcome in. 5. A subsequent run for one of the dropped task ids lands on the same warm worker before it recycles. The runtime workers now set a sentinel file context (`<no-context>`) around the `executor.execute(...)` call, so `task()` invocations firing during a run register normally. The catalog detects the sentinel and emits a one-time `console.warn` per task id so the pattern stays visible without spamming. The indexer never sets this context, so deploy-time behavior is unchanged. Repro is `references/hello-world/src/trigger/dynamicImportRepro*.ts`. Verified end-to-end against a deployed image with firestarter warm-starts on: pre-fix saw `COULD_NOT_FIND_EXECUTOR` on children that landed on the parent-poisoned worker; post-fix all 23/23 runs succeeded and the warning surfaces in the parent's run trace.
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@trigger.dev/core": patch
|
||||
"trigger.dev": patch
|
||||
---
|
||||
|
||||
Fix `COULD_NOT_FIND_EXECUTOR` when a task's definition is loaded via `await import(...)` from inside another task's `run()`. The runtime workers now register such tasks with a sentinel file context, and the catalog logs a one-time warning per task id.
|
||||
@@ -47,6 +47,7 @@ import {
|
||||
SharedRuntimeManager,
|
||||
OtelTaskLogger,
|
||||
populateEnv,
|
||||
NO_FILE_CONTEXT,
|
||||
StandardLifecycleHooksManager,
|
||||
StandardLocalsManager,
|
||||
StandardMetadataManager,
|
||||
@@ -501,8 +502,8 @@ const zodIpc = new ZodIpcConnection({
|
||||
async () => {
|
||||
const beforeImport = performance.now();
|
||||
resourceCatalog.setCurrentFileContext(
|
||||
taskManifest.entryPoint,
|
||||
taskManifest.filePath
|
||||
taskManifest.filePath,
|
||||
taskManifest.entryPoint
|
||||
);
|
||||
|
||||
// Load init file if it exists
|
||||
@@ -610,6 +611,12 @@ const zodIpc = new ZodIpcConnection({
|
||||
|
||||
const signal = AbortSignal.any([_cancelController.signal, timeoutController.signal]);
|
||||
|
||||
// Sentinel context so `task()` calls firing during run / lifecycle
|
||||
// hooks (e.g. via `await import(...)` of a module containing a task
|
||||
// definition) register normally instead of being silently dropped.
|
||||
// Cleared in the surrounding finally below.
|
||||
resourceCatalog.setCurrentFileContext(NO_FILE_CONTEXT, NO_FILE_CONTEXT);
|
||||
|
||||
const { result } = await executor.execute(execution, ctx, signal);
|
||||
|
||||
if (_isRunning && !_isCancelled) {
|
||||
@@ -628,6 +635,7 @@ const zodIpc = new ZodIpcConnection({
|
||||
}
|
||||
} finally {
|
||||
standardHeartbeatsManager.stopHeartbeat();
|
||||
resourceCatalog.clearCurrentFileContext();
|
||||
|
||||
_execution = undefined;
|
||||
_isRunning = false;
|
||||
|
||||
@@ -47,6 +47,7 @@ import {
|
||||
OtelTaskLogger,
|
||||
populateEnv,
|
||||
ProdUsageManager,
|
||||
NO_FILE_CONTEXT,
|
||||
StandardLifecycleHooksManager,
|
||||
StandardLocalsManager,
|
||||
StandardMetadataManager,
|
||||
@@ -490,8 +491,8 @@ const zodIpc = new ZodIpcConnection({
|
||||
async () => {
|
||||
const beforeImport = performance.now();
|
||||
resourceCatalog.setCurrentFileContext(
|
||||
taskManifest.entryPoint,
|
||||
taskManifest.filePath
|
||||
taskManifest.filePath,
|
||||
taskManifest.entryPoint
|
||||
);
|
||||
|
||||
// Load init file if it exists
|
||||
@@ -595,6 +596,12 @@ const zodIpc = new ZodIpcConnection({
|
||||
|
||||
const signal = AbortSignal.any([_cancelController.signal, timeoutController.signal]);
|
||||
|
||||
// Sentinel context so `task()` calls firing during run / lifecycle
|
||||
// hooks (e.g. via `await import(...)` of a module containing a task
|
||||
// definition) register normally instead of being silently dropped.
|
||||
// Cleared in the surrounding finally below.
|
||||
resourceCatalog.setCurrentFileContext(NO_FILE_CONTEXT, NO_FILE_CONTEXT);
|
||||
|
||||
const { result } = await executor.execute(execution, ctx, signal);
|
||||
|
||||
if (_isRunning && !_isCancelled) {
|
||||
@@ -613,6 +620,7 @@ const zodIpc = new ZodIpcConnection({
|
||||
}
|
||||
} finally {
|
||||
standardHeartbeatsManager.stopHeartbeat();
|
||||
resourceCatalog.clearCurrentFileContext();
|
||||
|
||||
_execution = undefined;
|
||||
_isRunning = false;
|
||||
|
||||
@@ -12,6 +12,18 @@ import {
|
||||
import { PromptMetadataWithFunctions, TaskMetadataWithFunctions, TaskSchema } from "../types/index.js";
|
||||
import { ResourceCatalog } from "./catalog.js";
|
||||
|
||||
/**
|
||||
* Sentinel file-context value the runtime workers set around task execution
|
||||
* (via `TaskExecutor.execute`) so that `task()` calls firing during a run —
|
||||
* e.g. as a side effect of `await import(...)` of a module containing a
|
||||
* task definition — register normally instead of hitting the silent-drop
|
||||
* guard in `registerTaskMetadata`. The catalog uses this exact string to
|
||||
* detect "registered during execution" and emit a one-time warning per
|
||||
* task id. The indexer never sets this context, so its behavior is
|
||||
* unchanged.
|
||||
*/
|
||||
export const NO_FILE_CONTEXT = "<no-context>";
|
||||
|
||||
export class StandardResourceCatalog implements ResourceCatalog {
|
||||
private _taskSchemas: Map<string, TaskSchema> = new Map();
|
||||
private _taskMetadata: Map<string, TaskMetadata> = new Map();
|
||||
@@ -25,6 +37,7 @@ export class StandardResourceCatalog implements ResourceCatalog {
|
||||
private _queueMetadata: Map<string, QueueManifest> = new Map();
|
||||
private _skillMetadata: Map<string, SkillMetadata> = new Map();
|
||||
private _skillFileMetadata: Map<string, TaskFileMetadata> = new Map();
|
||||
private _sentinelContextWarned: Set<string> = new Set();
|
||||
|
||||
setCurrentFileContext(filePath: string, entryPoint: string) {
|
||||
this._currentFileContext = { filePath, entryPoint };
|
||||
@@ -77,6 +90,20 @@ export class StandardResourceCatalog implements ResourceCatalog {
|
||||
return;
|
||||
}
|
||||
|
||||
// When the current context is the sentinel set by TaskExecutor around a
|
||||
// run, the task() call fired during execution — most commonly via a
|
||||
// dynamic import inside another task's run(). Warn once per task id so
|
||||
// the pattern stays visible.
|
||||
if (
|
||||
this._currentFileContext.filePath === NO_FILE_CONTEXT &&
|
||||
!this._sentinelContextWarned.has(task.id)
|
||||
) {
|
||||
this._sentinelContextWarned.add(task.id);
|
||||
console.warn(
|
||||
`[trigger.dev] task "${task.id}" was registered via dynamic import during another task's run(); move to a static import if you notice any issues.`
|
||||
);
|
||||
}
|
||||
|
||||
this._taskFileMetadata.set(task.id, {
|
||||
...this._currentFileContext,
|
||||
});
|
||||
|
||||
@@ -10,7 +10,10 @@ export {
|
||||
recordSpanException,
|
||||
carrierFromContext,
|
||||
} from "../otel/index.js";
|
||||
export { StandardResourceCatalog } from "../resource-catalog/standardResourceCatalog.js";
|
||||
export {
|
||||
StandardResourceCatalog,
|
||||
NO_FILE_CONTEXT,
|
||||
} from "../resource-catalog/standardResourceCatalog.js";
|
||||
export {
|
||||
TaskContextSpanProcessor,
|
||||
TaskContextLogProcessor,
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
// Fixture mimicking a task entrypoint file: top-level code calls into the
|
||||
// catalog (the same way `task()` / `schemaTask()` does via
|
||||
// `registerTaskMetadata`).
|
||||
//
|
||||
// Loaded via `await import()` from inside a test that simulates the worker
|
||||
// running a task. The point is to exercise top-level evaluation through Node's
|
||||
// ESM module loader so the module-cache semantics are real.
|
||||
|
||||
const register = globalThis.__catalogRegisterTaskMetadata;
|
||||
if (typeof register === "function") {
|
||||
register({
|
||||
id: "lazy-task",
|
||||
fns: {
|
||||
run: async () => "ok",
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
export const lazyTask = { id: "lazy-task" };
|
||||
@@ -0,0 +1,154 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
NO_FILE_CONTEXT,
|
||||
StandardResourceCatalog,
|
||||
} from "../src/v3/resource-catalog/standardResourceCatalog.js";
|
||||
|
||||
// Regression tests for COULD_NOT_FIND_EXECUTOR on warm worker processes when
|
||||
// a task's `task()` / `schemaTask()` call is evaluated during another task's
|
||||
// execution (e.g. as a side effect of `await import(...)` of a module that
|
||||
// contains a task definition).
|
||||
//
|
||||
// Production throw site:
|
||||
// - managed-run-worker.ts:566 (post-wrap)
|
||||
// - dev-run-worker.ts:578 (post-wrap)
|
||||
// Pre-fix symptom: `resourceCatalog.getTask(execution.task.id)` returned
|
||||
// undefined even after the worker re-imported the task entrypoint.
|
||||
//
|
||||
// Pre-fix mechanism: `registerTaskMetadata` silently returned when
|
||||
// `_currentFileContext` was unset. Any `task()` call firing during a
|
||||
// running task's run() / lifecycle hooks (directly, or transitively via a
|
||||
// dynamic import) hit the silent guard. Node's ESM module cache then
|
||||
// prevented recovery — the worker's setContext + re-import fallback didn't
|
||||
// re-evaluate the module body, so the `task()` call never fired again.
|
||||
//
|
||||
// Post-fix: the runtime workers wrap their `executor.execute(...)` call with
|
||||
// `setCurrentFileContext(NO_FILE_CONTEXT, NO_FILE_CONTEXT)` so any `task()`
|
||||
// call firing during execution registers normally with sentinel file
|
||||
// metadata. The catalog detects the sentinel and emits a one-time warning
|
||||
// per task id to keep the bundle-shape pattern visible. The indexer never
|
||||
// sets this sentinel context — its behavior is unchanged.
|
||||
|
||||
describe("StandardResourceCatalog — runtime registration via sentinel context", () => {
|
||||
afterEach(() => {
|
||||
delete (globalThis as { __catalogRegisterTaskMetadata?: unknown })
|
||||
.__catalogRegisterTaskMetadata;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("silently drops registration when no context is set (indexer's invariant)", () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const catalog = new StandardResourceCatalog();
|
||||
|
||||
catalog.registerTaskMetadata({
|
||||
id: "no-context-task",
|
||||
fns: { run: async () => "ok" },
|
||||
});
|
||||
|
||||
expect(catalog.getTask("no-context-task")).toBeUndefined();
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it(
|
||||
"registers normally and warns once when the sentinel context is set " +
|
||||
"(simulates the worker's executor wrap)",
|
||||
() => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const catalog = new StandardResourceCatalog();
|
||||
|
||||
catalog.setCurrentFileContext(NO_FILE_CONTEXT, NO_FILE_CONTEXT);
|
||||
catalog.registerTaskMetadata({
|
||||
id: "lazy-task",
|
||||
fns: { run: async () => "ok" },
|
||||
});
|
||||
catalog.clearCurrentFileContext();
|
||||
|
||||
const registered = catalog.getTask("lazy-task");
|
||||
expect(registered).toBeDefined();
|
||||
expect(registered?.id).toBe("lazy-task");
|
||||
expect(registered?.filePath).toBe(NO_FILE_CONTEXT);
|
||||
expect(registered?.entryPoint).toBe(NO_FILE_CONTEXT);
|
||||
expect(warn).toHaveBeenCalledTimes(1);
|
||||
expect(warn.mock.calls[0]?.[0]).toContain("lazy-task");
|
||||
}
|
||||
);
|
||||
|
||||
it(
|
||||
"warm-start path: a task whose top-level definition fires during a " +
|
||||
"dynamic import inside the sentinel wrap remains findable; the " +
|
||||
"worker's setContext + re-import fallback (managed-run-worker.ts:482) " +
|
||||
"is not needed",
|
||||
async () => {
|
||||
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const catalog = new StandardResourceCatalog();
|
||||
|
||||
(globalThis as { __catalogRegisterTaskMetadata?: unknown })
|
||||
.__catalogRegisterTaskMetadata = (
|
||||
task: Parameters<StandardResourceCatalog["registerTaskMetadata"]>[0]
|
||||
) => {
|
||||
catalog.registerTaskMetadata(task);
|
||||
};
|
||||
|
||||
// Simulate the worker wrap: setContext(NO_FILE_CONTEXT) → run user code
|
||||
// (which does a dynamic import) → clearContext.
|
||||
catalog.setCurrentFileContext(NO_FILE_CONTEXT, NO_FILE_CONTEXT);
|
||||
await import("./fixtures/dynamic-task-module.mjs");
|
||||
catalog.clearCurrentFileContext();
|
||||
|
||||
const registered = catalog.getTask("lazy-task");
|
||||
expect(registered).toBeDefined();
|
||||
expect(registered?.filePath).toBe(NO_FILE_CONTEXT);
|
||||
}
|
||||
);
|
||||
|
||||
it("warns at most once per task id under the sentinel context", () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const catalog = new StandardResourceCatalog();
|
||||
|
||||
catalog.setCurrentFileContext(NO_FILE_CONTEXT, NO_FILE_CONTEXT);
|
||||
|
||||
const register = (id: string) =>
|
||||
catalog.registerTaskMetadata({
|
||||
id,
|
||||
fns: { run: async () => "ok" },
|
||||
});
|
||||
|
||||
register("task-a");
|
||||
register("task-a");
|
||||
register("task-a");
|
||||
expect(warn).toHaveBeenCalledTimes(1);
|
||||
|
||||
register("task-b");
|
||||
expect(warn).toHaveBeenCalledTimes(2);
|
||||
|
||||
catalog.clearCurrentFileContext();
|
||||
});
|
||||
|
||||
it(
|
||||
"control: real file context registers without firing the sentinel warning",
|
||||
async () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const catalog = new StandardResourceCatalog();
|
||||
|
||||
(globalThis as { __catalogRegisterTaskMetadata?: unknown })
|
||||
.__catalogRegisterTaskMetadata = (
|
||||
task: Parameters<StandardResourceCatalog["registerTaskMetadata"]>[0]
|
||||
) => {
|
||||
catalog.registerTaskMetadata(task);
|
||||
};
|
||||
|
||||
catalog.setCurrentFileContext(
|
||||
"/app/dist/lazy-task.entry.mjs",
|
||||
"src/tasks/lazy-task.ts"
|
||||
);
|
||||
await import("./fixtures/dynamic-task-module.mjs?control");
|
||||
catalog.clearCurrentFileContext();
|
||||
|
||||
const task = catalog.getTask("lazy-task");
|
||||
expect(task).toBeDefined();
|
||||
expect(task?.filePath).toBe("/app/dist/lazy-task.entry.mjs");
|
||||
expect(task?.entryPoint).toBe("src/tasks/lazy-task.ts");
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
}
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,12 @@
|
||||
import { task } from "@trigger.dev/sdk";
|
||||
|
||||
// Defined in a module that's loaded via `await import(...)` from the parent
|
||||
// task's run() function. Pre-fix: the task() call below fires while
|
||||
// `_currentFileContext` is unset, so the registration is silently dropped.
|
||||
// Post-fix: registered with sentinel file metadata + console.warn fires once.
|
||||
export const lazyChildTask = task({
|
||||
id: "lazy-child-task",
|
||||
run: async (payload: { value: string }) => {
|
||||
return { received: payload.value };
|
||||
},
|
||||
});
|
||||
@@ -0,0 +1,17 @@
|
||||
import { logger, task } from "@trigger.dev/sdk";
|
||||
|
||||
// Triggers the dynamic-import silent-drop path. The child task's `task()`
|
||||
// definition lives in a module loaded via `await import(...)` inside this
|
||||
// parent's run() — so its registration would land outside the worker's
|
||||
// cold-load context window.
|
||||
export const dynamicImportReproParent = task({
|
||||
id: "dynamic-import-repro-parent",
|
||||
run: async () => {
|
||||
logger.info("parent: about to dynamically import child task module");
|
||||
const { lazyChildTask } = await import("./dynamicImportReproChild.js");
|
||||
logger.info("parent: import complete, triggering child");
|
||||
const handle = await lazyChildTask.trigger({ value: "hello from parent" });
|
||||
logger.info("parent: child triggered", { childRunId: handle.id });
|
||||
return { childRunId: handle.id };
|
||||
},
|
||||
});
|
||||
Reference in New Issue
Block a user