From 20615bbb13ac57c83594cfc770c9ddefcfcb1b86 Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Tue, 17 Mar 2026 13:37:15 +0800 Subject: [PATCH] fix(plugins): correct JSDoc for callHandlerWithTimeout timeout behavior --- src/plugins/hooks.ts | 89 ++++++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 23 deletions(-) diff --git a/src/plugins/hooks.ts b/src/plugins/hooks.ts index e8e1e2aa163..fb4faa0986d 100644 --- a/src/plugins/hooks.ts +++ b/src/plugins/hooks.ts @@ -5,6 +5,7 @@ * error handling, priority ordering, and async support. */ +import { withTimeout } from "../node-host/with-timeout.js"; import { concatOptionalTextSegments } from "../shared/text/join-segments.js"; import type { PluginRegistry } from "./registry.js"; import type { @@ -108,10 +109,15 @@ export type HookRunnerLogger = { error: (message: string) => void; }; +/** Default timeout for async plugin hook handlers (ms). */ +const DEFAULT_HOOK_TIMEOUT_MS = 10_000; + export type HookRunnerOptions = { logger?: HookRunnerLogger; /** If true, errors in hooks will be caught and logged instead of thrown */ catchErrors?: boolean; + /** Per-handler timeout for async hooks (ms). Defaults to 10 000. Set 0 to disable. */ + hookTimeoutMs?: number; }; export type PluginTargetedInboundClaimOutcome = @@ -159,6 +165,27 @@ function getHooksForNameAndPlugin( export function createHookRunner(registry: PluginRegistry, options: HookRunnerOptions = {}) { const logger = options.logger; const catchErrors = options.catchErrors ?? true; + const hookTimeoutMs = + typeof options.hookTimeoutMs === "number" && options.hookTimeoutMs > 0 + ? options.hookTimeoutMs + : options.hookTimeoutMs === 0 + ? undefined // explicitly disabled + : DEFAULT_HOOK_TIMEOUT_MS; + + /** + * Execute a single async handler with the configured timeout. + * Throws on timeout; callers catch via handleHookError. + */ + async function callHandlerWithTimeout( + fn: () => Promise, + hookName: PluginHookName, + pluginId: string, + ): Promise { + if (!hookTimeoutMs) { + return fn(); + } + return withTimeout(() => fn(), hookTimeoutMs, `${hookName} handler from ${pluginId}`); + } const mergeBeforeModelResolve = ( acc: PluginHookBeforeModelResolveResult | undefined, @@ -253,7 +280,11 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp const promises = hooks.map(async (hook) => { try { - await (hook.handler as (event: unknown, ctx: unknown) => Promise)(event, ctx); + await callHandlerWithTimeout( + () => (hook.handler as (event: unknown, ctx: unknown) => Promise)(event, ctx), + hookName, + hook.pluginId, + ); } catch (err) { handleHookError({ hookName, pluginId: hook.pluginId, error: err }); } @@ -283,9 +314,11 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp for (const hook of hooks) { try { - const handlerResult = await ( - hook.handler as (event: unknown, ctx: unknown) => Promise - )(event, ctx); + const handlerResult = await callHandlerWithTimeout( + () => (hook.handler as (event: unknown, ctx: unknown) => Promise)(event, ctx), + hookName, + hook.pluginId, + ); if (handlerResult !== undefined && handlerResult !== null) { if (mergeResults && result !== undefined) { @@ -317,7 +350,23 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp logger?.debug?.(`[hooks] running ${hookName} (${hooks.length} handlers, first-claim wins)`); - return await runClaimingHooksList(hooks, hookName, event, ctx); + for (const hook of hooks) { + try { + const handlerResult = await callHandlerWithTimeout( + () => + (hook.handler as (event: unknown, ctx: unknown) => Promise)(event, ctx), + hookName, + hook.pluginId, + ); + if (handlerResult?.handled) { + return handlerResult; + } + } catch (err) { + handleHookError({ hookName, pluginId: hook.pluginId, error: err }); + } + } + + return undefined; } async function runClaimingHookForPlugin< @@ -338,23 +387,14 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp `[hooks] running ${hookName} for ${pluginId} (${hooks.length} handlers, targeted)`, ); - return await runClaimingHooksList(hooks, hookName, event, ctx); - } - - async function runClaimingHooksList< - K extends PluginHookName, - TResult extends { handled: boolean }, - >( - hooks: Array & { pluginId: string }>, - hookName: K, - event: Parameters["handler"]>>[0], - ctx: Parameters["handler"]>>[1], - ): Promise { for (const hook of hooks) { try { - const handlerResult = await ( - hook.handler as (event: unknown, ctx: unknown) => Promise - )(event, ctx); + const handlerResult = await callHandlerWithTimeout( + () => + (hook.handler as (event: unknown, ctx: unknown) => Promise)(event, ctx), + hookName, + hook.pluginId, + ); if (handlerResult?.handled) { return handlerResult; } @@ -400,9 +440,12 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp let firstError: string | null = null; for (const hook of hooks) { try { - const handlerResult = await ( - hook.handler as (event: unknown, ctx: unknown) => Promise - )(event, ctx); + const handlerResult = await callHandlerWithTimeout( + () => + (hook.handler as (event: unknown, ctx: unknown) => Promise)(event, ctx), + hookName, + hook.pluginId, + ); if (handlerResult?.handled) { return { status: "handled", result: handlerResult }; }