From e33b71f9564cce2c988100e10fb180c44c5e6133 Mon Sep 17 00:00:00 2001 From: Sean Derrow Date: Mon, 5 Oct 2026 01:16:49 -0400 Subject: [PATCH] format feedback --- src/Job.ts | 22 ++++++++++++++------- src/async-context.ts | 20 ++----------------- src/schedule-stack.ts | 28 +++++++++++++++++++++++++- test/schedule-stack.test.ts | 39 ++++++++++++++++++++++++++++++++----- tsdown.config.mts | 6 +++--- 5 files changed, 81 insertions(+), 34 deletions(-) diff --git a/src/Job.ts b/src/Job.ts index 5814315..c46d43d 100644 --- a/src/Job.ts +++ b/src/Job.ts @@ -3,11 +3,16 @@ import type Bottleneck from "./Bottleneck"; import type Events from "./Events"; import type States from "./States"; import type { EventInfo, EventInfoRetryable, JobDefaults, ResolvedJobOptions } from "./types"; -import { bindTask, captureAsyncResource, runWithAsyncResource } from "./async-context"; +import { captureAsyncResource, runWithAsyncResource } from "./async-context"; import BottleneckError from "./BottleneckError"; import { load } from "./parser"; import randomIndex from "./random-index"; -import { attachScheduleStack, forwardChain, type ScheduleStackCapture } from "./schedule-stack"; +import { + attachScheduleStack, + forwardChain, + runTask, + type ScheduleStackCapture, +} from "./schedule-stack"; const NUM_PRIORITIES = 10; const DEFAULT_PRIORITY = 5; @@ -28,9 +33,10 @@ class Job { */ scheduleStackCapture: ScheduleStackCapture | undefined; /** - * Async context active when schedule() was called, re-entered for the - * chained limiter's schedule() path in doExecute (which otherwise runs - * from this job's timer context). Undefined outside Node. + * Async context active when schedule() was called (e.g. AsyncLocalStorage + * state), re-entered in doExecute to run the task or forward it to the + * chained limiter, which otherwise run from this job's timer context. + * Undefined outside Node, where the task runs without it. * @internal */ asyncResource: AsyncResource | undefined; @@ -49,7 +55,7 @@ class Job { _states: States, scheduleStackCapture?: ScheduleStackCapture, ) { - this.task = bindTask(task); + this.task = task; this.args = args; this.rejectOnDrop = rejectOnDrop; this.Events = Events; @@ -160,7 +166,9 @@ class Job { chained.schedule(this.options as never, this.task as never, ...(this.args ?? [])), ), ) - : (this.task as (...args: never[]) => unknown)(...(this.args ?? []))); + : this.asyncResource != null + ? this.asyncResource.runInAsyncScope(runTask, this, this.task, this.args ?? []) + : this.task(...(this.args ?? []))); if (clearGlobalState()) { this.doDone(eventInfo); diff --git a/src/async-context.ts b/src/async-context.ts index dcd2992..6df70e4 100644 --- a/src/async-context.ts +++ b/src/async-context.ts @@ -1,26 +1,10 @@ import { AsyncResource } from "node:async_hooks"; -/** - * Bind a scheduled task to the async context (e.g. AsyncLocalStorage state) - * active when schedule() is called, so the task observes that context when it - * eventually runs — no matter which tick, timer, or queued drain executes it. - * - * Falls back to the unbound task outside Node (the light/browser build stubs - * this module out entirely). - */ -export function bindTask unknown>(task: T): T { - try { - return AsyncResource.bind(task as T & ((...args: unknown[]) => unknown)); - } catch { - return task; - } -} - /** * Capture the async context (e.g. AsyncLocalStorage state) active right now * as a resource that can be re-entered later from a foreign async context. - * Used for the chained limiter's schedule() path in `Job.doExecute`, which - * runs from this job's timer context instead of the schedule-time context. + * `Job` captures one at schedule() and runs its task inside it, so the task + * observes that context no matter which tick, timer, or drain executes it. * * Returns undefined outside Node (the light/browser build shims * `node:async_hooks` without an AsyncResource constructor). diff --git a/src/schedule-stack.ts b/src/schedule-stack.ts index e7c7b9b..0ec03d1 100644 --- a/src/schedule-stack.ts +++ b/src/schedule-stack.ts @@ -1,5 +1,25 @@ +// The frame runTask adds. Only the sync stack carries it: after an await in +// the task, the capture ends in `at async Job.doExecute` instead. +const TASK_RUNNER_FRAME = /^\s*at (?:\S+\.)?__bottleneckRunTask\b/; + +// A method, since minifiers keep property names but rename functions. +const taskRunner = { + __bottleneckRunTask(this: unknown, task: (...args: never[]) => unknown, args: never[]): unknown { + return task.apply(this, args); + }, +}; + /** - * Strip the "Error" headline from a captured stack, leaving only the frames. + * Call `task`. Its frame marks where library frames begin in the schedule + * stack of a job scheduled from inside the task. Pass it straight to + * `runInAsyncScope` so it replaces that call's wrapper frame instead of + * adding one. + */ +export const runTask = taskRunner.__bottleneckRunTask; + +/** + * Strip the "Error" headline from a captured stack, leaving only the frames, + * and the library frames below a task that scheduled the job. */ function cleanScheduleStack(raw: string | undefined): string | undefined { if (typeof raw !== "string") { @@ -15,6 +35,12 @@ function cleanScheduleStack(raw: string | undefined): string | undefined { if (lines.length === 0) { return undefined; } + // Scheduled from inside a task: the frames from the task runner down are + // the library running that task, which the next section stands in for. + const runner = lines.findIndex((line) => TASK_RUNNER_FRAME.test(line)); + if (runner > 0) { + lines.length = runner; + } return lines.join("\n"); } diff --git a/test/schedule-stack.test.ts b/test/schedule-stack.test.ts index 794988d..7816d4b 100644 --- a/test/schedule-stack.test.ts +++ b/test/schedule-stack.test.ts @@ -26,6 +26,12 @@ function expectNoLibraryFrames(section: string | undefined): void { expect(section).not.toMatch(/src[/\\](Bottleneck|Job)\.ts/); } +/** Frames that ran the task a job was scheduled from, cut at its marker. */ +function expectNoTaskRunnerFrames(section: string | undefined): void { + expect(section).toBeDefined(); + expect(section).not.toMatch(/__bottleneckRunTask|runInAsyncScope|bound \[as task\]/); +} + describe("Schedule stacks", () => { test("rejection stack includes both the task and the schedule call site", async ({ makeLimiter, @@ -154,6 +160,8 @@ describe("Schedule stacks", () => { // Only the outermost section reaches the app's original call site. expect(sections[2]).toContain("startNestedSchedules"); expect(sections[0]).not.toContain("startNestedSchedules"); + expectNoTaskRunnerFrames(sections[0]); + expectNoTaskRunnerFrames(sections[1]); }); test("wrap() and withOptions() capture the wrapped function's caller", async ({ @@ -276,12 +284,12 @@ describe("Schedule stacks", () => { const inner = makeLimiter({ id: "task-body-inner", datastore: "local" }); const outer = makeLimiter({ id: "task-body-outer", datastore: "local" }); + const outerTaskBody = () => + inner.schedule(() => { + throw new Error("boom-task-body"); + }); function scheduleFromTaskBody(): Promise { - return outer.schedule(() => - inner.schedule(() => { - throw new Error("boom-task-body"); - }), - ) as Promise; + return outer.schedule(outerTaskBody) as Promise; } const error = await scheduleFromTaskBody().catch((e: unknown) => e); @@ -291,6 +299,27 @@ describe("Schedule stacks", () => { expect(stack).toContain("task-body-inner):"); expect(stack).toContain("task-body-outer):"); expect(stack).toContain("scheduleFromTaskBody"); + const [innerSection] = scheduleSections(error); + expect(innerSection).toContain("outerTaskBody"); + expectNoTaskRunnerFrames(innerSection); + }); + + test("a schedule after an await in a task body keeps the task frame", async ({ makeLimiter }) => { + const inner = makeLimiter({ id: "task-await-inner", datastore: "local" }); + const outer = makeLimiter({ id: "task-await-outer", datastore: "local" }); + + const outerTaskAwait = async () => { + await Promise.resolve(); + return inner.schedule(() => { + throw new Error("boom-task-await"); + }); + }; + const error = await outer.schedule(outerTaskAwait).catch((e: unknown) => e); + const [innerSection] = scheduleSections(error); + expect(innerSection).toContain("outerTaskAwait"); + // The marker isn't on the stack after the await; V8 leaves just one + // `at async Job.doExecute` frame below the task. + expectNoTaskRunnerFrames(innerSection); }); test("an explicit scheduleStackLabel is used for the marker", async ({ makeLimiter }) => { diff --git a/tsdown.config.mts b/tsdown.config.mts index e7ded02..f6e6e2a 100644 --- a/tsdown.config.mts +++ b/tsdown.config.mts @@ -47,8 +47,8 @@ const excludeClustering: TsdownPlugin = { }; // `src/async-context.ts` statically imports `node:async_hooks`, which has no -// browser equivalent. The light build redirects that import to a shim whose -// `bind` is the identity function, so tasks simply run unbound. +// browser equivalent. The light build redirects that import to a shim with no +// AsyncResource constructor, so tasks simply run without async context. const shimAsyncHooks: TsdownPlugin = { name: "shim-async-hooks", resolveId(id) { @@ -57,7 +57,7 @@ const shimAsyncHooks: TsdownPlugin = { load(id) { if (id === "virtual:async-hooks-shim") { return { - code: "export const AsyncResource = { bind: (fn) => fn };", + code: "export const AsyncResource = {};", moduleType: "js", }; }