Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Fix issues with push connect reconnection #13085

Merged
merged 11 commits into from
Feb 6, 2025
1 change: 0 additions & 1 deletion packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,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
Loading