fix(core): Account for immediate confirmation request during test webhook creation (#8329)

This commit is contained in:
Iván Ovejero 2024-01-16 09:17:41 +01:00 committed by GitHub
parent dfa486e4f2
commit 5fbd7971e0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 10 deletions

View file

@ -23,6 +23,7 @@ import { WorkflowMissingIdError } from '@/errors/workflow-missing-id.error';
import { WebhookNotFoundError } from '@/errors/response-errors/webhook-not-found.error';
import * as NodeExecuteFunctions from 'n8n-core';
import { removeTrailingSlash } from './utils';
import type { TestWebhookRegistration } from '@/services/test-webhook-registrations.service';
import { TestWebhookRegistrationsService } from '@/services/test-webhook-registrations.service';
import { MultiMainSetup } from './services/orchestration/main/MultiMainSetup.ee';
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
@ -232,13 +233,13 @@ export class TestWebhooks implements IWebhookManager {
for (const webhook of webhooks) {
const key = this.registrations.toKey(webhook);
const registration = await this.registrations.get(key);
const isAlreadyRegistered = await this.registrations.get(key);
if (runData && webhook.node in runData) {
return false;
}
if (registration && !webhook.webhookId) {
if (isAlreadyRegistered && !webhook.webhookId) {
throw new WebhookPathTakenError(webhook.node);
}
@ -253,17 +254,25 @@ export class TestWebhooks implements IWebhookManager {
cacheableWebhook.userId = userId;
const registration: TestWebhookRegistration = {
sessionId,
workflowEntity,
destinationNode,
webhook: cacheableWebhook as IWebhookData,
};
try {
/**
* Register the test webhook _before_ creation at third-party service
* in case service sends a confirmation request immediately on creation.
*/
await this.registrations.register(registration);
await workflow.createWebhookIfNotExists(webhook, NodeExecuteFunctions, 'manual', 'manual');
cacheableWebhook.staticData = workflow.staticData;
await this.registrations.register({
sessionId,
workflowEntity,
destinationNode,
webhook: cacheableWebhook as IWebhookData,
});
await this.registrations.register(registration);
this.timeouts[key] = timeout;
} catch (error) {

View file

@ -8,7 +8,7 @@ import * as WebhookHelpers from '@/WebhookHelpers';
import type * as express from 'express';
import type { IWorkflowDb, WebhookRequest } from '@/Interfaces';
import type { IWebhookData, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import type { IWebhookData, IWorkflowExecuteAdditionalData, Workflow } from 'n8n-workflow';
import type {
TestWebhookRegistrationsService,
TestWebhookRegistration,
@ -50,11 +50,18 @@ describe('TestWebhooks', () => {
mock<IWorkflowExecuteAdditionalData>(),
];
test('if webhook is needed, should return true and activate webhook', async () => {
test('if webhook is needed, should register then create webhook and return true', async () => {
const workflow = mock<Workflow>();
jest.spyOn(testWebhooks, 'toWorkflow').mockReturnValueOnce(workflow);
jest.spyOn(WebhookHelpers, 'getWorkflowWebhooks').mockReturnValue([webhook]);
const needsWebhook = await testWebhooks.needsWebhook(...args);
const [registerOrder] = registrations.register.mock.invocationCallOrder;
const [createOrder] = workflow.createWebhookIfNotExists.mock.invocationCallOrder;
expect(registerOrder).toBeLessThan(createOrder);
expect(needsWebhook).toBe(true);
});