github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-10-19 16:55:02 +02:00
Code Issues Actions 13 Packages Projects Releases Wiki Activity Graph

fix(core): Release test-webhook isolate only after teardown completes (#36573)

Browse Source 
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Danny Martini <danny@n8n.io>
This commit is contained in:
Mike Repeć 2026-08-19 14:16:25 +00:00 committed by GitHub
parent b0cfa9391c
commit 05de34323c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 107 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

97
packages/cli/src/webhooks/__tests__/test-webhooks.test.ts
View File

@ -44,6 +44,12 @@ const webhook = mock<IWebhookData>({
userId,
});
const flushMicrotasks = async () => {
const { setImmediate: realSetImmediate } =
await vi.importActual<typeof import('timers')>('timers');
await new Promise((resolve) => realSetImmediate(resolve));
};
describe('TestWebhooks', () => {
const logger = mock<Logger>();
const registrations = mock<TestWebhookRegistrationsService>();
@ -426,6 +432,91 @@ describe('TestWebhooks', () => {
await expect(promise).rejects.toThrowError(WebhookNotFoundError);
});
test('releases isolate only after deactivateWebhooks completes on successful execution', async () => {
const expression = mock<WorkflowExpression>();
const workflowStartNode = mock<ReturnType<Workflow['getNode']>>({
type: 'n8n-nodes-base.noOp',
});
const workflow = mock<Workflow>({
id: workflowEntity.id,
expression,
getNode: vi.fn().mockReturnValue(workflowStartNode),
});
vi.spyOn(testWebhooks, 'toWorkflow').mockReturnValue(workflow);
vi.spyOn(testWebhooks, 'getActiveWebhook').mockResolvedValue(webhook);
registrations.get.mockResolvedValueOnce({
version: 1,
workflowEntity,
webhook,
} as TestWebhookRegistration);
const deactivateSpy = vi
.spyOn(testWebhooks, 'deactivateWebhooks')
.mockResolvedValue(undefined);
vi.spyOn(WebhookHelpers, 'executeWebhook').mockImplementation(async (...args: unknown[]) => {
const onDone = args[10] as (error: Error | null, data: unknown) => void;
onDone(null, { noWebhookResponse: true });
return 'execution-id';
});
await testWebhooks.executeWebhook(
mock<WebhookRequest>({ params: { path }, method: httpMethod }),
mock<express.Response>(),
);
await flushMicrotasks();
expect(deactivateSpy).toHaveBeenCalledWith(workflow);
expect(expression.acquireIsolate).toHaveBeenCalledTimes(1);
expect(expression.releaseIsolate).toHaveBeenCalledTimes(1);
const [acquireOrder] = (expression.acquireIsolate as Mock).mock.invocationCallOrder;
const [deactivateOrder] = deactivateSpy.mock.invocationCallOrder;
const [releaseOrder] = (expression.releaseIsolate as Mock).mock.invocationCallOrder;
expect(acquireOrder).toBeLessThan(deactivateOrder);
expect(deactivateOrder).toBeLessThan(releaseOrder);
});
test('logs when isolate release fails after teardown', async () => {
const expression = mock<WorkflowExpression>();
const workflowStartNode = mock<ReturnType<Workflow['getNode']>>({
type: 'n8n-nodes-base.noOp',
});
const workflow = mock<Workflow>({
id: workflowEntity.id,
expression,
getNode: vi.fn().mockReturnValue(workflowStartNode),
});
vi.spyOn(testWebhooks, 'toWorkflow').mockReturnValue(workflow);
vi.spyOn(testWebhooks, 'getActiveWebhook').mockResolvedValue(webhook);
registrations.get.mockResolvedValueOnce({
version: 1,
workflowEntity,
webhook,
} as TestWebhookRegistration);
vi.spyOn(testWebhooks, 'deactivateWebhooks').mockResolvedValue(undefined);
vi.spyOn(WebhookHelpers, 'executeWebhook').mockImplementation(async (...args: unknown[]) => {
const onDone = args[10] as (error: Error | null, data: unknown) => void;
onDone(null, { noWebhookResponse: true });
return 'execution-id';
});
const error = new Error('release failed');
expression.releaseIsolate.mockRejectedValueOnce(error);
await testWebhooks.executeWebhook(
mock<WebhookRequest>({ params: { path }, method: httpMethod }),
mock<express.Response>(),
);
await flushMicrotasks();
expect(logger.error).toHaveBeenCalledWith(
'Failed to release expression isolate for test webhook',
expect.objectContaining({ error, workflowId: workflowEntity.id }),
);
});
});
describe('deactivateWebhooks()', () => {
@ -446,12 +537,6 @@ describe('TestWebhooks', () => {
});
describe('cancelWebhook()', () => {
const flushMicrotasks = async () => {
const { setImmediate: realSetImmediate } =
await vi.importActual<typeof import('timers')>('timers');
await new Promise((resolve) => realSetImmediate(resolve));
};
test('acquires and releases isolate around deactivateWebhooks', async () => {
const expression = mock<WorkflowExpression>();
const workflow = mock<Workflow>({ id: workflowEntity.id, expression });

23
packages/cli/src/webhooks/test-webhooks.ts
View File

@ -146,8 +146,10 @@ export class TestWebhooks implements IWebhookManager {
}
await workflow.expression.acquireIsolate();
try {
return await new Promise(async (resolve, reject) => {
// Release only after teardown below runs, not when resolve() settles the
// promise early — teardown still needs the isolate held.
return await new Promise(async (resolve, reject) => {
try {
try {
const executionMode = 'manual';
const executionId = await WebhookHelpers.executeWebhook(
@ -172,7 +174,6 @@ export class TestWebhooks implements IWebhookManager {
// or a ping so do not resolve the promise and wait for the real webhook
// request instead.
if (executionId === undefined) {
await workflow.expression.releaseIsolate();
return;
}
@ -212,10 +213,18 @@ export class TestWebhooks implements IWebhookManager {
this.clearTimeout(key);
await this.deactivateWebhooks(workflow);
});
} finally {
await workflow.expression.releaseIsolate();
}
} finally {
// Response (if any) was already sent, so a release failure here can only be logged.
try {
await workflow.expression.releaseIsolate();
} catch (error) {
this.logger.error('Failed to release expression isolate for test webhook', {
error,
workflowId: workflow.id,
});
}
}
});
}
@OnPubSubEvent('clear-test-webhooks', { instanceType: 'main' })