Skip to content

Commit

Permalink
fix(editor): Fix issues with push connect reconnection (#13085)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomi authored Feb 6, 2025
1 parent e59d983 commit fff98b1
Show file tree
Hide file tree
Showing 19 changed files with 905 additions and 135 deletions.
1 change: 0 additions & 1 deletion packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,6 @@ export interface RootState {
endpointWebhook: string;
endpointWebhookTest: string;
endpointWebhookWaiting: string;
pushConnectionActive: boolean;
timezone: string;
executionTimeout: number;
maxExecutionTimeout: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ vi.mock('@/composables/useToast', () => {
},
};
});

vi.mock('@/stores/pushConnection.store', () => ({
usePushConnectionStore: vi.fn().mockReturnValue({
isConnected: true,
}),
}));

// Test data
const mockNodes: INodeUi[] = [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ vi.mock('vue-router', () => ({
RouterLink: vi.fn(),
}));

vi.mock('@/stores/pushConnection.store', () => ({
usePushConnectionStore: vi.fn().mockReturnValue({
isConnected: true,
}),
}));

const initialState = {
[STORES.SETTINGS]: {
settings: {
Expand Down
58 changes: 58 additions & 0 deletions packages/editor-ui/src/components/PushConnectionTracker.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { createComponentRenderer } from '@/__tests__/render';
import PushConnectionTracker from '@/components/PushConnectionTracker.vue';
import { STORES } from '@/constants';
import { createTestingPinia } from '@pinia/testing';
import { setActivePinia } from 'pinia';

let isConnected = true;
let isConnectionRequested = true;

vi.mock('@/stores/pushConnection.store', () => {
return {
usePushConnectionStore: vi.fn(() => ({
isConnected,
isConnectionRequested,
})),
};
});

describe('PushConnectionTracker', () => {
const render = () => {
const pinia = createTestingPinia({
stubActions: false,
initialState: {
[STORES.PUSH]: {
isConnected,
isConnectionRequested,
},
},
});
setActivePinia(pinia);

return createComponentRenderer(PushConnectionTracker)();
};

it('should not render error when connected and connection requested', () => {
isConnected = true;
isConnectionRequested = true;
const { container } = render();

expect(container).toMatchSnapshot();
});

it('should render error when disconnected and connection requested', () => {
isConnected = false;
isConnectionRequested = true;
const { container } = render();

expect(container).toMatchSnapshot();
});

it('should not render error when connected and connection not requested', () => {
isConnected = true;
isConnectionRequested = false;
const { container } = render();

expect(container).toMatchSnapshot();
});
});
17 changes: 12 additions & 5 deletions packages/editor-ui/src/components/PushConnectionTracker.vue
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
<script setup lang="ts">
import { computed } from 'vue';
import { useRootStore } from '@/stores/root.store';
import { usePushConnectionStore } from '@/stores/pushConnection.store';
import { useI18n } from '@/composables/useI18n';
import { computed } from 'vue';
const rootStore = useRootStore();
const pushConnectionActive = computed(() => rootStore.pushConnectionActive);
const pushConnectionStore = usePushConnectionStore();
const i18n = useI18n();
const showConnectionLostError = computed(() => {
// Only show the connection lost error if the connection has been requested
// and the connection is not currently connected. This is to prevent the
// connection error from being shown e.g. when user navigates directly to
// the workflow executions list, which doesn't open the connection.
return pushConnectionStore.isConnectionRequested && !pushConnectionStore.isConnected;
});
</script>

<template>
<span>
<div v-if="!pushConnectionActive" class="push-connection-lost primary-color">
<div v-if="showConnectionLostError" class="push-connection-lost primary-color">
<n8n-tooltip placement="bottom-end">
<template #content>
<div v-n8n-html="i18n.baseText('pushConnectionTracker.cannotConnectToServer')"></div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`PushConnectionTracker > should not render error when connected and connection not requested 1`] = `
<div>
<span>
</span>
</div>
`;

exports[`PushConnectionTracker > should not render error when connected and connection requested 1`] = `
<div>
<span>
</span>
</div>
`;

exports[`PushConnectionTracker > should render error when disconnected and connection requested 1`] = `
<div>
<span>
<div
class="push-connection-lost primary-color"
>
<span
class="el-tooltip__trigger"
>
<svg
aria-hidden="true"
class="svg-inline--fa fa-exclamation-triangle fa-w-18"
data-icon="exclamation-triangle"
data-prefix="fas"
focusable="false"
role="img"
viewBox="0 0 576 512"
xmlns="http://www.w3.org/2000/svg"
>
<path
class=""
d="M569.517 440.013C587.975 472.007 564.806 512 527.94 512H48.054c-36.937 0-59.999-40.055-41.577-71.987L246.423 23.985c18.467-32.009 64.72-31.951 83.154 0l239.94 416.028zM288 354c-25.405 0-46 20.595-46 46s20.595 46 46 46 46-20.595 46-46-20.595-46-46-46zm-43.673-165.346l7.418 136c.347 6.364 5.609 11.346 11.982 11.346h48.546c6.373 0 11.635-4.982 11.982-11.346l7.418-136c.375-6.874-5.098-12.654-11.982-12.654h-63.383c-6.884 0-12.356 5.78-11.981 12.654z"
fill="currentColor"
/>
</svg>
  Connection lost
</span>
<!--teleport start-->
<!--teleport end-->
</div>
</span>
</div>
`;
29 changes: 18 additions & 11 deletions packages/editor-ui/src/composables/useRunWorkflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
type ITaskData,
} from 'n8n-workflow';

import { useRootStore } from '@/stores/root.store';
import { useRunWorkflow } from '@/composables/useRunWorkflow';
import type { IStartRunData, IWorkflowData } from '@/Interface';
import { useWorkflowsStore } from '@/stores/workflows.store';
Expand All @@ -21,6 +20,7 @@ import { useToast } from './useToast';
import { useI18n } from '@/composables/useI18n';
import { captor, mock } from 'vitest-mock-extended';
import { useSettingsStore } from '@/stores/settings.store';
import { usePushConnectionStore } from '@/stores/pushConnection.store';

vi.mock('@/stores/workflows.store', () => ({
useWorkflowsStore: vi.fn().mockReturnValue({
Expand All @@ -40,6 +40,12 @@ vi.mock('@/stores/workflows.store', () => ({
}),
}));

vi.mock('@/stores/pushConnection.store', () => ({
usePushConnectionStore: vi.fn().mockReturnValue({
isConnected: true,
}),
}));

vi.mock('@/composables/useTelemetry', () => ({
useTelemetry: vi.fn().mockReturnValue({ track: vi.fn() }),
}));
Expand Down Expand Up @@ -90,7 +96,7 @@ vi.mock('vue-router', async (importOriginal) => {
});

describe('useRunWorkflow({ router })', () => {
let rootStore: ReturnType<typeof useRootStore>;
let pushConnectionStore: ReturnType<typeof usePushConnectionStore>;
let uiStore: ReturnType<typeof useUIStore>;
let workflowsStore: ReturnType<typeof useWorkflowsStore>;
let router: ReturnType<typeof useRouter>;
Expand All @@ -102,7 +108,7 @@ describe('useRunWorkflow({ router })', () => {

setActivePinia(pinia);

rootStore = useRootStore();
pushConnectionStore = usePushConnectionStore();
uiStore = useUIStore();
workflowsStore = useWorkflowsStore();
settingsStore = useSettingsStore();
Expand All @@ -120,7 +126,7 @@ describe('useRunWorkflow({ router })', () => {
it('should throw an error if push connection is not active', async () => {
const { runWorkflowApi } = useRunWorkflow({ router });

rootStore.setPushConnectionInactive();
vi.mocked(pushConnectionStore).isConnected = false;

await expect(runWorkflowApi({} as IStartRunData)).rejects.toThrow(
'workflowRun.noActiveConnectionToTheServer',
Expand All @@ -130,7 +136,7 @@ describe('useRunWorkflow({ router })', () => {
it('should successfully run a workflow', async () => {
const { runWorkflowApi } = useRunWorkflow({ router });

rootStore.setPushConnectionActive();
vi.mocked(pushConnectionStore).isConnected = true;

const mockResponse = { executionId: '123', waitingForWebhook: false };
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockResponse);
Expand Down Expand Up @@ -161,7 +167,7 @@ describe('useRunWorkflow({ router })', () => {
it('should handle workflow run failure', async () => {
const { runWorkflowApi } = useRunWorkflow({ router });

rootStore.setPushConnectionActive();
vi.mocked(pushConnectionStore).isConnected = true;
vi.mocked(workflowsStore).runWorkflow.mockRejectedValue(new Error('Failed to run workflow'));

await expect(runWorkflowApi({} as IStartRunData)).rejects.toThrow('Failed to run workflow');
Expand All @@ -171,7 +177,7 @@ describe('useRunWorkflow({ router })', () => {
it('should set waitingForWebhook if response indicates waiting', async () => {
const { runWorkflowApi } = useRunWorkflow({ router });

rootStore.setPushConnectionActive();
vi.mocked(pushConnectionStore).isConnected = true;
const mockResponse = { executionId: '123', waitingForWebhook: true };
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockResponse);

Expand Down Expand Up @@ -210,6 +216,7 @@ describe('useRunWorkflow({ router })', () => {
type: 'error',
});
});

it('should execute workflow has pin data and is active with single webhook trigger', async () => {
const pinia = createTestingPinia({ stubActions: false });
setActivePinia(pinia);
Expand Down Expand Up @@ -295,7 +302,7 @@ describe('useRunWorkflow({ router })', () => {
const mockExecutionResponse = { executionId: '123' };
const { runWorkflow } = useRunWorkflow({ router });

vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(pushConnectionStore).isConnected = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue({
Expand Down Expand Up @@ -405,7 +412,7 @@ describe('useRunWorkflow({ router })', () => {
workflow.getParentNodes.mockReturnValue([]);

vi.mocked(settingsStore).partialExecutionVersion = 1;
vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(pushConnectionStore).isConnected = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue(workflow);
Expand Down Expand Up @@ -436,7 +443,7 @@ describe('useRunWorkflow({ router })', () => {
workflow.getParentNodes.mockReturnValue([]);

vi.mocked(settingsStore).partialExecutionVersion = 2;
vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(pushConnectionStore).isConnected = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue(workflow);
Expand Down Expand Up @@ -465,7 +472,7 @@ describe('useRunWorkflow({ router })', () => {
workflow.getParentNodes.mockReturnValue([]);

vi.mocked(settingsStore).partialExecutionVersion = 2;
vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(pushConnectionStore).isConnected = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue(workflow);
Expand Down
4 changes: 3 additions & 1 deletion packages/editor-ui/src/composables/useRunWorkflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { useI18n } from '@/composables/useI18n';
import { get } from 'lodash-es';
import { useExecutionsStore } from '@/stores/executions.store';
import { useSettingsStore } from '@/stores/settings.store';
import { usePushConnectionStore } from '@/stores/pushConnection.store';

const getDirtyNodeNames = (
runData: IRunData,
Expand Down Expand Up @@ -63,12 +64,13 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType<typeof u
const toast = useToast();

const rootStore = useRootStore();
const pushConnectionStore = usePushConnectionStore();
const uiStore = useUIStore();
const workflowsStore = useWorkflowsStore();
const executionsStore = useExecutionsStore();
// Starts to execute a workflow on server
async function runWorkflowApi(runData: IStartRunData): Promise<IExecutionPushResponse> {
if (!rootStore.pushConnectionActive) {
if (!pushConnectionStore.isConnected) {
// Do not start if the connection to server is not active
// because then it can not receive the data as it executes.
throw new Error(i18n.baseText('workflowRun.noActiveConnectionToTheServer'));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/** Mocked EventSource class to help testing */
export class MockEventSource extends EventTarget {
constructor(public url: string) {
super();
}

simulateConnectionOpen() {
this.dispatchEvent(new Event('open'));
}

simulateConnectionClose() {
this.dispatchEvent(new Event('close'));
}

simulateMessageEvent(data: string) {
this.dispatchEvent(new MessageEvent('message', { data }));
}

close = vi.fn();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { WebSocketState } from '@/push-connection/useWebSocketClient';

/** Mocked WebSocket class to help testing */
export class MockWebSocket extends EventTarget {
readyState: number = WebSocketState.CONNECTING;

constructor(public url: string) {
super();
}

simulateConnectionOpen() {
this.dispatchEvent(new Event('open'));
this.readyState = WebSocketState.OPEN;
}

simulateConnectionClose(code: number) {
this.dispatchEvent(new CloseEvent('close', { code }));
this.readyState = WebSocketState.CLOSED;
}

simulateMessageEvent(data: string) {
this.dispatchEvent(new MessageEvent('message', { data }));
}

dispatchErrorEvent() {
this.dispatchEvent(new Event('error'));
}

send = vi.fn();

close = vi.fn();
}
Loading

0 comments on commit fff98b1

Please sign in to comment.