mirror of
https://github.com/n8n-io/n8n.git
synced 2024-12-24 20:24:05 -08:00
refactor(core)!: Remove webhook deregistration at startup and shutdown (#7515)
https://linear.app/n8n/issue/PAY-932/deprecate-flag-to-skip-webhook-deregistration-on-shutdown
This commit is contained in:
parent
5477e3fb45
commit
ae8c7a635e
|
@ -2,6 +2,18 @@
|
||||||
|
|
||||||
This list shows all the versions which include breaking changes and how to upgrade.
|
This list shows all the versions which include breaking changes and how to upgrade.
|
||||||
|
|
||||||
|
## 1.15.0
|
||||||
|
|
||||||
|
### What changed?
|
||||||
|
|
||||||
|
Until now, in main mode, n8n used to deregister webhooks at shutdown and reregister them at startup. Queue mode and the flag `N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN` skipped webhook deregistration.
|
||||||
|
|
||||||
|
As from now, in both main and queue modes, n8n no longer deregisters webhooks at startup and shutdown, and the flag `N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN` is removed. n8n assumes that third-party services will retry unhandled webhook requests.
|
||||||
|
|
||||||
|
### When is action necessary?
|
||||||
|
|
||||||
|
If using the flag `N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN`, note that it no longer has effect and can be removed from your settings.
|
||||||
|
|
||||||
## 1.9.0
|
## 1.9.0
|
||||||
|
|
||||||
### What changed?
|
### What changed?
|
||||||
|
|
|
@ -47,7 +47,6 @@ import * as ResponseHelper from '@/ResponseHelper';
|
||||||
import * as WebhookHelpers from '@/WebhookHelpers';
|
import * as WebhookHelpers from '@/WebhookHelpers';
|
||||||
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
|
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
|
||||||
|
|
||||||
import config from '@/config';
|
|
||||||
import type { User } from '@db/entities/User';
|
import type { User } from '@db/entities/User';
|
||||||
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
|
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
|
||||||
import { ActiveExecutions } from '@/ActiveExecutions';
|
import { ActiveExecutions } from '@/ActiveExecutions';
|
||||||
|
@ -101,18 +100,6 @@ export class ActiveWorkflowRunner implements IWebhookManager {
|
||||||
relations: ['shared', 'shared.user', 'shared.user.globalRole', 'shared.role'],
|
relations: ['shared', 'shared.user', 'shared.user.globalRole', 'shared.role'],
|
||||||
})) as IWorkflowDb[];
|
})) as IWorkflowDb[];
|
||||||
|
|
||||||
if (!config.getEnv('endpoints.skipWebhooksDeregistrationOnShutdown')) {
|
|
||||||
// Do not clean up database when skip registration is done.
|
|
||||||
// This flag is set when n8n is running in scaled mode.
|
|
||||||
// Impact is minimal, but for a short while, n8n will stop accepting requests.
|
|
||||||
// Also, users had issues when running multiple "main process"
|
|
||||||
// instances if many of them start at the same time
|
|
||||||
// This is not officially supported but there is no reason
|
|
||||||
// it should not work.
|
|
||||||
// Clear up active workflow table
|
|
||||||
await this.webhookService.deleteInstanceWebhooks();
|
|
||||||
}
|
|
||||||
|
|
||||||
if (workflowsData.length !== 0) {
|
if (workflowsData.length !== 0) {
|
||||||
this.logger.info(' ================================');
|
this.logger.info(' ================================');
|
||||||
this.logger.info(' Start Active Workflows:');
|
this.logger.info(' Start Active Workflows:');
|
||||||
|
@ -404,12 +391,7 @@ export class ActiveWorkflowRunner implements IWebhookManager {
|
||||||
false,
|
false,
|
||||||
);
|
);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (
|
if (activation === 'init' && error.name === 'QueryFailedError') {
|
||||||
activation === 'init' &&
|
|
||||||
config.getEnv('endpoints.skipWebhooksDeregistrationOnShutdown') &&
|
|
||||||
error.name === 'QueryFailedError'
|
|
||||||
) {
|
|
||||||
// When skipWebhooksDeregistrationOnShutdown is enabled,
|
|
||||||
// n8n does not remove the registered webhooks on exit.
|
// n8n does not remove the registered webhooks on exit.
|
||||||
// This means that further initializations will always fail
|
// This means that further initializations will always fail
|
||||||
// when inserting to database. This is why we ignore this error
|
// when inserting to database. This is why we ignore this error
|
||||||
|
|
|
@ -284,12 +284,4 @@ export class TestWebhooks implements IWebhookManager {
|
||||||
|
|
||||||
return foundWebhook;
|
return foundWebhook;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Removes all the currently active test webhooks
|
|
||||||
*/
|
|
||||||
async removeAll(): Promise<void> {
|
|
||||||
const workflows = Object.values(this.testWebhookData).map(({ workflow }) => workflow);
|
|
||||||
return this.activeWebhooks.removeAll(workflows);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -74,6 +74,12 @@ export abstract class BaseCommand extends Command {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (process.env.N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN) {
|
||||||
|
this.logger.warn(
|
||||||
|
'The flag to skip webhook deregistration N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN has been removed. n8n no longer deregisters webhooks at startup and shutdown, in main and queue mode.',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
await Container.get(PostHogClient).init();
|
await Container.get(PostHogClient).init();
|
||||||
await Container.get(InternalHooks).init();
|
await Container.get(InternalHooks).init();
|
||||||
}
|
}
|
||||||
|
|
|
@ -21,7 +21,6 @@ import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
|
||||||
import * as Db from '@/Db';
|
import * as Db from '@/Db';
|
||||||
import * as GenericHelpers from '@/GenericHelpers';
|
import * as GenericHelpers from '@/GenericHelpers';
|
||||||
import { Server } from '@/Server';
|
import { Server } from '@/Server';
|
||||||
import { TestWebhooks } from '@/TestWebhooks';
|
|
||||||
import { EDITOR_UI_DIST_DIR, GENERATED_STATIC_DIR } from '@/constants';
|
import { EDITOR_UI_DIST_DIR, GENERATED_STATIC_DIR } from '@/constants';
|
||||||
import { eventBus } from '@/eventbus';
|
import { eventBus } from '@/eventbus';
|
||||||
import { BaseCommand } from './BaseCommand';
|
import { BaseCommand } from './BaseCommand';
|
||||||
|
@ -115,21 +114,6 @@ export class Start extends BaseCommand {
|
||||||
|
|
||||||
await Container.get(InternalHooks).onN8nStop();
|
await Container.get(InternalHooks).onN8nStop();
|
||||||
|
|
||||||
const skipWebhookDeregistration = config.getEnv(
|
|
||||||
'endpoints.skipWebhooksDeregistrationOnShutdown',
|
|
||||||
);
|
|
||||||
|
|
||||||
const removePromises = [];
|
|
||||||
if (!skipWebhookDeregistration) {
|
|
||||||
removePromises.push(this.activeWorkflowRunner.removeAll());
|
|
||||||
}
|
|
||||||
|
|
||||||
// Remove all test webhooks
|
|
||||||
const testWebhooks = Container.get(TestWebhooks);
|
|
||||||
removePromises.push(testWebhooks.removeAll());
|
|
||||||
|
|
||||||
await Promise.all(removePromises);
|
|
||||||
|
|
||||||
// Wait for active workflow executions to finish
|
// Wait for active workflow executions to finish
|
||||||
const activeExecutionsInstance = Container.get(ActiveExecutions);
|
const activeExecutionsInstance = Container.get(ActiveExecutions);
|
||||||
let executingWorkflows = activeExecutionsInstance.getActiveExecutions();
|
let executingWorkflows = activeExecutionsInstance.getActiveExecutions();
|
||||||
|
|
|
@ -107,12 +107,6 @@ export class WebhookService {
|
||||||
return this.deleteWebhooks(webhooks);
|
return this.deleteWebhooks(webhooks);
|
||||||
}
|
}
|
||||||
|
|
||||||
async deleteInstanceWebhooks() {
|
|
||||||
const webhooks = await this.webhookRepository.find();
|
|
||||||
|
|
||||||
return this.deleteWebhooks(webhooks);
|
|
||||||
}
|
|
||||||
|
|
||||||
private async deleteWebhooks(webhooks: WebhookEntity[]) {
|
private async deleteWebhooks(webhooks: WebhookEntity[]) {
|
||||||
void this.cacheService.deleteMany(webhooks.map((w) => w.cacheKey));
|
void this.cacheService.deleteMany(webhooks.map((w) => w.cacheKey));
|
||||||
|
|
||||||
|
|
|
@ -160,7 +160,6 @@ describe('ActiveWorkflowRunner', () => {
|
||||||
await activeWorkflowRunner.init();
|
await activeWorkflowRunner.init();
|
||||||
expect(await activeWorkflowRunner.getActiveWorkflows()).toHaveLength(0);
|
expect(await activeWorkflowRunner.getActiveWorkflows()).toHaveLength(0);
|
||||||
expect(mocked(Db.collections.Workflow.find)).toHaveBeenCalled();
|
expect(mocked(Db.collections.Workflow.find)).toHaveBeenCalled();
|
||||||
expect(webhookService.deleteInstanceWebhooks).toHaveBeenCalled();
|
|
||||||
expect(externalHooks.run).toHaveBeenCalledTimes(1);
|
expect(externalHooks.run).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -171,7 +170,6 @@ describe('ActiveWorkflowRunner', () => {
|
||||||
databaseActiveWorkflowsCount,
|
databaseActiveWorkflowsCount,
|
||||||
);
|
);
|
||||||
expect(mocked(Db.collections.Workflow.find)).toHaveBeenCalled();
|
expect(mocked(Db.collections.Workflow.find)).toHaveBeenCalled();
|
||||||
expect(webhookService.deleteInstanceWebhooks).toHaveBeenCalled();
|
|
||||||
expect(externalHooks.run).toHaveBeenCalled();
|
expect(externalHooks.run).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
@ -150,30 +150,6 @@ describe('WebhookService', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('deleteInstanceWebhooks()', () => {
|
|
||||||
test('should delete all webhooks of the instance', async () => {
|
|
||||||
const mockInstanceWebhooks = [
|
|
||||||
createWebhook('PUT', 'users'),
|
|
||||||
createWebhook('GET', 'user/:id'),
|
|
||||||
createWebhook('POST', ':var'),
|
|
||||||
];
|
|
||||||
|
|
||||||
webhookRepository.find.mockResolvedValue(mockInstanceWebhooks);
|
|
||||||
|
|
||||||
await webhookService.deleteInstanceWebhooks();
|
|
||||||
|
|
||||||
expect(webhookRepository.remove).toHaveBeenCalledWith(mockInstanceWebhooks);
|
|
||||||
});
|
|
||||||
|
|
||||||
test('should not delete any webhooks if none found', async () => {
|
|
||||||
webhookRepository.find.mockResolvedValue([]);
|
|
||||||
|
|
||||||
await webhookService.deleteInstanceWebhooks();
|
|
||||||
|
|
||||||
expect(webhookRepository.remove).toHaveBeenCalledWith([]);
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
describe('deleteWorkflowWebhooks()', () => {
|
describe('deleteWorkflowWebhooks()', () => {
|
||||||
test('should delete all webhooks of the workflow', async () => {
|
test('should delete all webhooks of the workflow', async () => {
|
||||||
const mockWorkflowWebhooks = [
|
const mockWorkflowWebhooks = [
|
||||||
|
|
Loading…
Reference in a new issue