From cd58b049adf870e250ddbb1c70c73c820ac16ca4 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 19 May 2026 23:58:21 +0200 Subject: [PATCH 1/3] fix(workflow-executor): suppress spurious timeout log when step fails before timeout fires Co-Authored-By: Claude Sonnet 4.6 --- .../src/executors/base-step-executor.ts | 10 +++++- .../test/executors/base-step-executor.test.ts | 33 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/workflow-executor/src/executors/base-step-executor.ts b/packages/workflow-executor/src/executors/base-step-executor.ts index 8f6de37859..08bc535216 100644 --- a/packages/workflow-executor/src/executors/base-step-executor.ts +++ b/packages/workflow-executor/src/executors/base-step-executor.ts @@ -173,9 +173,14 @@ export default abstract class BaseStepExecutor { + if (!timeoutFired) return; this.context.logger.info('Step work rejected after timeout — result discarded', { runId: this.context.runId, stepId: this.context.stepId, @@ -188,7 +193,10 @@ export default abstract class BaseStepExecutor((_, reject) => { - timer = setTimeout(() => reject(new StepTimeoutError(timeoutMs)), timeoutMs); + timer = setTimeout(() => { + timeoutFired = true; + reject(new StepTimeoutError(timeoutMs)); + }, timeoutMs); }), ]); } finally { diff --git a/packages/workflow-executor/test/executors/base-step-executor.test.ts b/packages/workflow-executor/test/executors/base-step-executor.test.ts index 1b0f16bb1e..fe21dc8e7f 100644 --- a/packages/workflow-executor/test/executors/base-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/base-step-executor.test.ts @@ -453,6 +453,39 @@ describe('BaseStepExecutor', () => { process.off('unhandledRejection', unhandled); } }, 5_000); + + it('does not log when execPromise rejects before the timeout fires', async () => { + class ImmediateFailExecutor extends BaseStepExecutor { + protected async doExecute(): Promise { + throw new Error('normal step error'); + } + + protected buildOutcomeResult(outcome: { + status: BaseStepStatus; + error?: string; + }): StepExecutionResult { + return { + stepOutcome: { + type: 'record', + stepId: this.context.stepId, + stepIndex: this.context.stepIndex, + status: outcome.status, + ...(outcome.error !== undefined && { error: outcome.error }), + }, + }; + } + } + + const logger = makeMockLogger(); + const executor = new ImmediateFailExecutor(makeContext({ stepTimeoutMs: 5_000, logger })); + + await executor.execute(); + + expect(logger.info).not.toHaveBeenCalledWith( + 'Step work rejected after timeout — result discarded', + expect.anything(), + ); + }); }); describe('activity log lifecycle', () => { From 191ddf052cadcaa344c23283dcc2bc30a18cdac5 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 20 May 2026 00:06:55 +0200 Subject: [PATCH 2/3] refactor(workflow-executor): replace ImmediateFailExecutor with TestableExecutor in timeout test Co-Authored-By: Claude Sonnet 4.6 --- .../test/executors/base-step-executor.test.ts | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/packages/workflow-executor/test/executors/base-step-executor.test.ts b/packages/workflow-executor/test/executors/base-step-executor.test.ts index fe21dc8e7f..7aca23a756 100644 --- a/packages/workflow-executor/test/executors/base-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/base-step-executor.test.ts @@ -455,29 +455,11 @@ describe('BaseStepExecutor', () => { }, 5_000); it('does not log when execPromise rejects before the timeout fires', async () => { - class ImmediateFailExecutor extends BaseStepExecutor { - protected async doExecute(): Promise { - throw new Error('normal step error'); - } - - protected buildOutcomeResult(outcome: { - status: BaseStepStatus; - error?: string; - }): StepExecutionResult { - return { - stepOutcome: { - type: 'record', - stepId: this.context.stepId, - stepIndex: this.context.stepIndex, - status: outcome.status, - ...(outcome.error !== undefined && { error: outcome.error }), - }, - }; - } - } - const logger = makeMockLogger(); - const executor = new ImmediateFailExecutor(makeContext({ stepTimeoutMs: 5_000, logger })); + const executor = new TestableExecutor( + makeContext({ stepTimeoutMs: 5_000, logger }), + new Error('normal step error'), + ); await executor.execute(); From d0a50549f6ec3dd84af854791615d71f698712ab Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 20 May 2026 12:16:10 +0200 Subject: [PATCH 3/3] refactor(workflow-executor): rename timeoutFired to hasTimeoutFired, trim comment, rename test Co-Authored-By: Claude Sonnet 4.6 --- .../src/executors/base-step-executor.ts | 9 +++------ .../test/executors/base-step-executor.test.ts | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/workflow-executor/src/executors/base-step-executor.ts b/packages/workflow-executor/src/executors/base-step-executor.ts index 08bc535216..885d680a3f 100644 --- a/packages/workflow-executor/src/executors/base-step-executor.ts +++ b/packages/workflow-executor/src/executors/base-step-executor.ts @@ -173,14 +173,11 @@ export default abstract class BaseStepExecutor { - if (!timeoutFired) return; + if (!hasTimeoutFired) return; this.context.logger.info('Step work rejected after timeout — result discarded', { runId: this.context.runId, stepId: this.context.stepId, @@ -194,7 +191,7 @@ export default abstract class BaseStepExecutor((_, reject) => { timer = setTimeout(() => { - timeoutFired = true; + hasTimeoutFired = true; reject(new StepTimeoutError(timeoutMs)); }, timeoutMs); }), diff --git a/packages/workflow-executor/test/executors/base-step-executor.test.ts b/packages/workflow-executor/test/executors/base-step-executor.test.ts index 7aca23a756..6cd5c51af0 100644 --- a/packages/workflow-executor/test/executors/base-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/base-step-executor.test.ts @@ -454,7 +454,7 @@ describe('BaseStepExecutor', () => { } }, 5_000); - it('does not log when execPromise rejects before the timeout fires', async () => { + it('does not log discard message when step rejects before timeout', async () => { const logger = makeMockLogger(); const executor = new TestableExecutor( makeContext({ stepTimeoutMs: 5_000, logger }),