Skip to content

Commit 2ad8d01

Browse files
Flexiconclaude
andauthored
fix(core): Stop task broker before task runner processes on shutdown (#37833)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 8f3617e commit 2ad8d01

2 files changed

Lines changed: 62 additions & 8 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import type { Logger } from '@n8n/backend-common';
2+
import type { TaskRunnersConfig } from '@n8n/config';
3+
import type { ErrorReporter } from 'n8n-core';
4+
import { mock } from 'vitest-mock-extended';
5+
6+
import type { EventService } from '@/events/event.service';
7+
import type { TaskBrokerServer } from '@/task-runners/task-broker/task-broker-server';
8+
import type { JsTaskRunnerProcess } from '@/task-runners/task-runner-process-js';
9+
import type { PyTaskRunnerProcess } from '@/task-runners/task-runner-process-py';
10+
import { TaskRunnerModule } from '@/task-runners/task-runner-module';
11+
12+
describe('TaskRunnerModule', () => {
13+
describe('stop', () => {
14+
it('should stop the broker server before the runner processes, so the drain can finish in-flight tasks first', async () => {
15+
const logger = mock<Logger>();
16+
logger.scoped.mockReturnValue(logger);
17+
const module = new TaskRunnerModule(
18+
logger,
19+
mock<ErrorReporter>(),
20+
mock<TaskRunnersConfig>(),
21+
mock<EventService>(),
22+
);
23+
24+
const brokerServer = mock<TaskBrokerServer>();
25+
const jsRunnerProcess = mock<JsTaskRunnerProcess>();
26+
const pyRunnerProcess = mock<PyTaskRunnerProcess>();
27+
Object.assign(module, {
28+
taskBrokerHttpServer: brokerServer,
29+
jsRunnerProcess,
30+
pyRunnerProcess,
31+
});
32+
33+
const order: string[] = [];
34+
// The broker stop completes asynchronously, so a stop() that merely starts
35+
// it without awaiting would record the runner stops first.
36+
brokerServer.stop.mockImplementation(async () => {
37+
await new Promise((resolve) => setImmediate(resolve));
38+
order.push('brokerServer');
39+
});
40+
jsRunnerProcess.stop.mockImplementation(async () => {
41+
order.push('jsRunnerProcess');
42+
});
43+
pyRunnerProcess.stop.mockImplementation(async () => {
44+
order.push('pyRunnerProcess');
45+
});
46+
47+
await module.stop();
48+
49+
expect(order).toHaveLength(3);
50+
expect(order[0]).toBe('brokerServer');
51+
});
52+
});
53+
});

packages/cli/src/task-runners/task-runner-module.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ export class TaskRunnerModule {
6767

6868
@OnShutdown()
6969
async stop() {
70+
// Stop the broker server first: its drain lets in-flight tasks finish, so the
71+
// runner processes are idle by the time they are stopped and exit within the
72+
// short grace before the SIGKILL escalation.
73+
if (this.taskBrokerHttpServer) {
74+
await this.taskBrokerHttpServer.stop();
75+
this.taskBrokerHttpServer = undefined;
76+
}
77+
7078
const stopRunnerProcessTask = (async () => {
7179
if (this.jsRunnerProcess) {
7280
await this.jsRunnerProcess.stop();
@@ -81,14 +89,7 @@ export class TaskRunnerModule {
8189
}
8290
})();
8391

84-
const stopRunnerServerTask = (async () => {
85-
if (this.taskBrokerHttpServer) {
86-
await this.taskBrokerHttpServer.stop();
87-
this.taskBrokerHttpServer = undefined;
88-
}
89-
})();
90-
91-
await Promise.all([stopRunnerProcessTask, stopPythonRunnerProcessTask, stopRunnerServerTask]);
92+
await Promise.all([stopRunnerProcessTask, stopPythonRunnerProcessTask]);
9293
}
9394

9495
private async loadTaskRequester() {

0 commit comments

Comments
 (0)