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(core): Bring back execution data on the executionFinished push message #11821

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 22 additions & 45 deletions cypress/utils/executions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { stringify } from 'flatted';
import type { IDataObject, IPinData, ITaskData, ITaskDataConnections } from 'n8n-workflow';
import type { IDataObject, ITaskData, ITaskDataConnections } from 'n8n-workflow';
import { nanoid } from 'nanoid';

import { clickExecuteWorkflowButton } from '../composables/workflow';

Expand Down Expand Up @@ -39,38 +40,6 @@ export function createMockNodeExecutionData(
};
}

function createMockWorkflowExecutionData({
runData,
lastNodeExecuted,
}: {
runData: Record<string, ITaskData | ITaskData[]>;
pinData?: IPinData;
lastNodeExecuted: string;
}) {
return {
data: stringify({
startData: {},
resultData: {
runData,
pinData: {},
lastNodeExecuted,
},
executionData: {
contextData: {},
nodeExecutionStack: [],
metadata: {},
waitingExecution: {},
waitingExecutionSource: {},
},
}),
mode: 'manual',
startedAt: new Date().toISOString(),
stoppedAt: new Date().toISOString(),
status: 'success',
finished: true,
};
}

export function runMockWorkflowExecution({
trigger,
lastNodeExecuted,
Expand All @@ -80,6 +49,7 @@ export function runMockWorkflowExecution({
lastNodeExecuted: string;
runData: Array<ReturnType<typeof createMockNodeExecutionData>>;
}) {
const workflowId = nanoid();
const executionId = Math.floor(Math.random() * 1_000_000).toString();

cy.intercept('POST', '/rest/workflows/**/run?**', {
Expand Down Expand Up @@ -117,17 +87,24 @@ export function runMockWorkflowExecution({
resolvedRunData[nodeName] = nodeExecution[nodeName];
});

cy.intercept('GET', `/rest/executions/${executionId}`, {
statusCode: 200,
body: {
data: createMockWorkflowExecutionData({
cy.push('executionFinished', {
executionId,
workflowId,
status: 'success',
rawData: stringify({
startData: {},
resultData: {
runData,
pinData: {},
lastNodeExecuted,
runData: resolvedRunData,
}),
},
}).as('getExecution');

cy.push('executionFinished', { executionId });

cy.wait('@getExecution');
},
executionData: {
contextData: {},
nodeExecutionStack: [],
metadata: {},
waitingExecution: {},
waitingExecutionSource: {},
},
}),
});
}
6 changes: 5 additions & 1 deletion packages/@n8n/api-types/src/push/execution.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ITaskData, WorkflowExecuteMode } from 'n8n-workflow';
import type { ExecutionStatus, ITaskData, WorkflowExecuteMode } from 'n8n-workflow';

type ExecutionStarted = {
type: 'executionStarted';
Expand All @@ -23,6 +23,10 @@ type ExecutionFinished = {
type: 'executionFinished';
data: {
executionId: string;
workflowId: string;
status: ExecutionStatus;
/** @deprecated: Please construct execution data in the frontend from the data pushed in previous messages, instead of depending on this additional payload serialization */
rawData?: string;
};
};

Expand Down
15 changes: 12 additions & 3 deletions packages/cli/src/workflow-execute-additional-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
import type { PushType } from '@n8n/api-types';
import { GlobalConfig } from '@n8n/config';
import { stringify } from 'flatted';
import { WorkflowExecute } from 'n8n-core';
import {
ApplicationError,
Expand Down Expand Up @@ -318,9 +319,17 @@ function hookFunctionsPush(): IWorkflowExecuteHooks {
workflowId,
});

const pushType =
fullRunData.status === 'waiting' ? 'executionWaiting' : 'executionFinished';
pushInstance.send(pushType, { executionId }, pushRef);
const { status } = fullRunData;
if (status === 'waiting') {
pushInstance.send('executionWaiting', { executionId }, pushRef);
} else {
const rawData = stringify(fullRunData.data);
pushInstance.send(
'executionFinished',
{ executionId, workflowId, status, rawData },
pushRef,
);
}
},
],
};
Expand Down
55 changes: 21 additions & 34 deletions packages/editor-ui/src/composables/usePushConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { stringify } from 'flatted';
import { useRouter } from 'vue-router';
import { createPinia, setActivePinia } from 'pinia';
import type { PushMessage, PushPayload } from '@n8n/api-types';
import { mock } from 'vitest-mock-extended';
import type { ITaskData, WorkflowOperationError } from 'n8n-workflow';

import { usePushConnection } from '@/composables/usePushConnection';
Expand All @@ -11,7 +10,6 @@ import { useOrchestrationStore } from '@/stores/orchestration.store';
import { useUIStore } from '@/stores/ui.store';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useToast } from '@/composables/useToast';
import type { IExecutionResponse } from '@/Interface';

vi.mock('vue-router', () => {
return {
Expand Down Expand Up @@ -140,39 +138,31 @@ describe('usePushConnection()', () => {

describe('executionFinished', () => {
const executionId = '1';
const event: PushMessage = {
type: 'executionFinished',
data: { executionId: '1' },
};
const workflowId = 'abc';

beforeEach(() => {
workflowsStore.activeExecutionId = executionId;
uiStore.isActionActive.workflowRunning = true;
});

it('should handle executionFinished event correctly', async () => {
const spy = vi.spyOn(workflowsStore, 'fetchExecutionDataById').mockResolvedValue(
mock<IExecutionResponse>({
id: executionId,
data: stringify({
const result = await pushConnection.pushMessageReceived({
type: 'executionFinished',
data: {
executionId,
workflowId,
status: 'success',
rawData: stringify({
resultData: {
runData: {},
},
}) as unknown as IExecutionResponse['data'],
finished: true,
mode: 'manual',
startedAt: new Date(),
stoppedAt: new Date(),
status: 'success',
}),
);

const result = await pushConnection.pushMessageReceived(event);
}),
},
});

expect(result).toBeTruthy();
expect(workflowsStore.workflowExecutionData).toBeDefined();
expect(uiStore.isActionActive['workflowRunning']).toBeTruthy();
expect(spy).toHaveBeenCalledWith(executionId);

expect(toast.showMessage).toHaveBeenCalledWith({
title: 'Workflow executed successfully',
Expand All @@ -181,10 +171,13 @@ describe('usePushConnection()', () => {
});

it('should handle isManualExecutionCancelled correctly', async () => {
const spy = vi.spyOn(workflowsStore, 'fetchExecutionDataById').mockResolvedValue(
mock<IExecutionResponse>({
id: executionId,
data: stringify({
const result = await pushConnection.pushMessageReceived({
type: 'executionFinished',
data: {
executionId,
workflowId,
status: 'error',
rawData: stringify({
startData: {},
resultData: {
runData: {
Expand All @@ -198,14 +191,9 @@ describe('usePushConnection()', () => {
node: 'Last Node',
} as unknown as WorkflowOperationError,
},
}) as unknown as IExecutionResponse['data'],
mode: 'manual',
startedAt: new Date(),
status: 'running',
}),
);

const result = await pushConnection.pushMessageReceived(event);
}),
},
});

expect(useToast().showMessage).toHaveBeenCalledWith({
message:
Expand All @@ -219,7 +207,6 @@ describe('usePushConnection()', () => {
expect(result).toBeTruthy();
expect(workflowsStore.workflowExecutionData).toBeDefined();
expect(uiStore.isActionActive.workflowRunning).toBeTruthy();
expect(spy).toHaveBeenCalledWith(executionId);
});
});

Expand Down
32 changes: 19 additions & 13 deletions packages/editor-ui/src/composables/usePushConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { useTelemetry } from '@/composables/useTelemetry';
import type { PushMessageQueueItem } from '@/types';
import { useAssistantStore } from '@/stores/assistant.store';
import NodeExecutionErrorMessage from '@/components/NodeExecutionErrorMessage.vue';
import type { IExecutionResponse } from '@/Interface';

export function usePushConnection({ router }: { router: ReturnType<typeof useRouter> }) {
const workflowHelpers = useWorkflowHelpers({ router });
Expand Down Expand Up @@ -205,11 +206,19 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
return false;
}

// pull execution data for the execution from the server
const executionData = await workflowsStore.fetchExecutionDataById(executionId);
if (!executionData?.data) return false;
// data comes in as 'flatten' object, so we need to parse it
executionData.data = parse(executionData.data as unknown as string) as IRunExecutionData;
let executionData: Pick<IExecutionResponse, 'workflowId' | 'data' | 'status'>;
if (receivedData.type === 'executionFinished' && receivedData.data.rawData) {
const { workflowId, status, rawData } = receivedData.data;
executionData = { workflowId, data: parse(rawData), status };
} else {
const execution = await workflowsStore.fetchExecutionDataById(executionId);
if (!execution?.data) return false;
executionData = {
workflowId: execution.workflowId,
data: parse(execution.data as unknown as string),
status: execution.status,
};
}

const iRunExecutionData: IRunExecutionData = {
startData: executionData.data?.startData,
Expand Down Expand Up @@ -265,7 +274,7 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou

// Workflow did start but had been put to wait
workflowHelpers.setDocumentTitle(workflow.name as string, 'IDLE');
} else if (executionData.finished !== true) {
} else if (executionData.status === 'error' || executionData.status === 'canceled') {
despairblue marked this conversation as resolved.
Show resolved Hide resolved
workflowHelpers.setDocumentTitle(workflow.name as string, 'ERROR');

if (
Expand Down Expand Up @@ -347,17 +356,14 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
duration: 0,
});
} else {
let title: string;
const isManualExecutionCancelled =
executionData.mode === 'manual' && executionData.status === 'canceled';

// Do not show the error message if the workflow got canceled manually
if (isManualExecutionCancelled) {
// Do not show the error message if the workflow got canceled
if (executionData.status === 'canceled') {
toast.showMessage({
title: i18n.baseText('nodeView.showMessage.stopExecutionTry.title'),
type: 'success',
});
} else {
let title: string;
if (iRunExecutionData.resultData.lastNodeExecuted) {
title = `Problem in node ‘${iRunExecutionData.resultData.lastNodeExecuted}‘`;
} else {
Expand Down Expand Up @@ -421,7 +427,7 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
}

workflowsStore.executingNode.length = 0;
workflowsStore.setWorkflowExecutionData(executionData);
workflowsStore.setWorkflowExecutionData(executionData as IExecutionResponse);
uiStore.removeActiveAction('workflowRunning');

// Set the node execution issues on all the nodes which produced an error so that
Expand Down
Loading