Skip to content

Commit

Permalink
fix: Always register webhooks on startup (#8830)
Browse files Browse the repository at this point in the history
  • Loading branch information
krynble authored Mar 7, 2024
1 parent c8d589c commit c6f6254
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
6 changes: 5 additions & 1 deletion packages/cli/src/services/orchestration.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ export class OrchestrationService {
* Whether this instance may add webhooks to the `webhook_entity` table.
*/
shouldAddWebhooks(activationMode: WorkflowActivateMode) {
if (activationMode === 'init') return false;
// Always try to populate the webhook entity table as well as register the webhooks
// to prevent issues with users upgrading from a version < 1.15, where the webhook entity
// was cleared on shutdown to anything past 1.28.0, where we stopped populating it on init,
// causing all webhooks to break
if (activationMode === 'init') return true;

if (activationMode === 'leadershipChange') return false;

Expand Down
35 changes: 35 additions & 0 deletions packages/cli/test/unit/services/orchestration.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Logger } from '@/Logger';
import { Push } from '@/push';
import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
import { mockInstance } from '../../shared/mocking';
import type { WorkflowActivateMode } from 'n8n-workflow';

const os = Container.get(OrchestrationService);
const handler = Container.get(OrchestrationHandlerMainService);
Expand Down Expand Up @@ -149,4 +150,38 @@ describe('Orchestration Service', () => {
expect(res1!.payload).toBeUndefined();
expect(res2!.payload!.result).toEqual('debounced');
});

describe('shouldAddWebhooks', () => {
beforeEach(() => {
config.set('multiMainSetup.instanceType', 'leader');
});
test('should return true for init', () => {
// We want to ensure that webhooks are populated on init
// more https://github.com/n8n-io/n8n/pull/8830
const result = os.shouldAddWebhooks('init');
expect(result).toBe(true);
});

test('should return false for leadershipChange', () => {
const result = os.shouldAddWebhooks('leadershipChange');
expect(result).toBe(false);
});

test('should return true for update or activate when is leader', () => {
const modes = ['update', 'activate'] as WorkflowActivateMode[];
for (const mode of modes) {
const result = os.shouldAddWebhooks(mode);
expect(result).toBe(true);
}
});

test('should return false for update or activate when not leader', () => {
config.set('multiMainSetup.instanceType', 'follower');
const modes = ['update', 'activate'] as WorkflowActivateMode[];
for (const mode of modes) {
const result = os.shouldAddWebhooks(mode);
expect(result).toBe(false);
}
});
});
});

0 comments on commit c6f6254

Please sign in to comment.