mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-28 07:17:04 +02:00
fix(editor): Provide better output for subnode execution errors (#21714)
This commit is contained in:
parent
b1fb445a3d
commit
5b2d15e78d
|
|
@ -170,7 +170,7 @@ export function makeHandleToolInvocation(
|
|||
} catch (error) {
|
||||
// Check if error is due to cancellation
|
||||
if (abortSignal?.aborted) {
|
||||
return 'Error during node execution: Execution was cancelled';
|
||||
throw new NodeOperationError(node, 'Execution was cancelled');
|
||||
}
|
||||
|
||||
const nodeError = new NodeOperationError(node, error as Error);
|
||||
|
|
@ -178,14 +178,26 @@ export function makeHandleToolInvocation(
|
|||
|
||||
lastError = nodeError;
|
||||
|
||||
// If this is the last attempt, throw the error
|
||||
// If this is the last attempt, throw the error to properly terminate execution
|
||||
if (tryIndex === maxTries - 1) {
|
||||
return 'Error during node execution: ' + (nodeError.description ?? nodeError.message);
|
||||
// Enhance the error with detailed information
|
||||
if (nodeError.description && !nodeError.message.includes(nodeError.description)) {
|
||||
nodeError.message = `${nodeError.message}\n\nDetails: ${nodeError.description}`;
|
||||
}
|
||||
throw nodeError;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return 'Error during node execution : ' + (lastError?.description ?? lastError?.message);
|
||||
// This should never be reached, but if it is, throw the error
|
||||
if (lastError) {
|
||||
if (lastError.description && !lastError.message.includes(lastError.description)) {
|
||||
lastError.message = `${lastError.message}\n\nDetails: ${lastError.description}`;
|
||||
}
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
throw new NodeOperationError(node, 'Unknown error during node execution');
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -352,6 +364,14 @@ export async function getInputConnectionData(
|
|||
currentNodeRunIndex,
|
||||
);
|
||||
|
||||
await context.addExecutionDataFunctions(
|
||||
'output',
|
||||
error,
|
||||
connectionType,
|
||||
parentNode.name,
|
||||
currentNodeRunIndex,
|
||||
);
|
||||
|
||||
// Display on the calling node which node has the error
|
||||
throw new NodeOperationError(connectedNode, `Error in sub-node ${connectedNode.name}`, {
|
||||
itemIndex,
|
||||
|
|
|
|||
|
|
@ -2,13 +2,23 @@ import { screen, waitFor, within } from '@testing-library/vue';
|
|||
import { createTestingPinia } from '@pinia/testing';
|
||||
import { h, defineComponent } from 'vue';
|
||||
import { useToast } from './useToast';
|
||||
import { useTelemetry } from './useTelemetry';
|
||||
import { vi } from 'vitest';
|
||||
|
||||
vi.mock('./useTelemetry');
|
||||
|
||||
describe('useToast', () => {
|
||||
let toast: ReturnType<typeof useToast>;
|
||||
let telemetryTrackSpy: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
createTestingPinia();
|
||||
|
||||
telemetryTrackSpy = vi.fn();
|
||||
vi.mocked(useTelemetry).mockReturnValue({
|
||||
track: telemetryTrackSpy,
|
||||
} as unknown as ReturnType<typeof useTelemetry>);
|
||||
|
||||
toast = useToast();
|
||||
});
|
||||
|
||||
|
|
@ -86,4 +96,112 @@ describe('useToast', () => {
|
|||
expect(screen.getByRole('alert')).toContainHTML('<p>Test <strong>content</strong></p>');
|
||||
});
|
||||
});
|
||||
|
||||
describe('telemetry tracking for error messages', () => {
|
||||
it('should track telemetry with string message for error toast', async () => {
|
||||
const messageData = {
|
||||
message: 'Error occurred',
|
||||
title: 'Error',
|
||||
type: 'error' as const,
|
||||
};
|
||||
|
||||
toast.showMessage(messageData);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(telemetryTrackSpy).toHaveBeenCalledWith('Instance FE emitted error', {
|
||||
error_title: 'Error',
|
||||
error_message: 'Error occurred',
|
||||
caused_by_credential: false,
|
||||
workflow_id: expect.any(String),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('should extract error message from VNode props for telemetry', async () => {
|
||||
const vnode = h(
|
||||
defineComponent({
|
||||
props: ['errorMessage', 'nodeName'],
|
||||
template: '<p>{{ errorMessage }}</p>',
|
||||
}),
|
||||
{
|
||||
errorMessage: 'Node execution failed',
|
||||
nodeName: 'TestNode',
|
||||
},
|
||||
);
|
||||
|
||||
const messageData = {
|
||||
message: vnode,
|
||||
title: 'Error in node',
|
||||
type: 'error' as const,
|
||||
};
|
||||
|
||||
toast.showMessage(messageData);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(telemetryTrackSpy).toHaveBeenCalledWith('Instance FE emitted error', {
|
||||
error_title: 'Error in node',
|
||||
error_message: 'Node execution failed',
|
||||
caused_by_credential: false,
|
||||
workflow_id: expect.any(String),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('should use "Unknown error" when VNode has no error message in props', async () => {
|
||||
const vnode = h(
|
||||
defineComponent({
|
||||
template: '<p>Some content</p>',
|
||||
}),
|
||||
);
|
||||
|
||||
const messageData = {
|
||||
message: vnode,
|
||||
title: 'Error',
|
||||
type: 'error' as const,
|
||||
};
|
||||
|
||||
toast.showMessage(messageData);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(telemetryTrackSpy).toHaveBeenCalledWith('Instance FE emitted error', {
|
||||
error_title: 'Error',
|
||||
error_message: 'Unknown error',
|
||||
caused_by_credential: false,
|
||||
workflow_id: expect.any(String),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('should not track telemetry for non-error messages', async () => {
|
||||
const messageData = {
|
||||
message: 'Success message',
|
||||
title: 'Success',
|
||||
type: 'success' as const,
|
||||
};
|
||||
|
||||
toast.showMessage(messageData);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('alert')).toBeVisible();
|
||||
});
|
||||
|
||||
expect(telemetryTrackSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not track telemetry when track parameter is false', async () => {
|
||||
const messageData = {
|
||||
message: 'Error occurred',
|
||||
title: 'Error',
|
||||
type: 'error' as const,
|
||||
};
|
||||
|
||||
toast.showMessage(messageData, false);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('alert')).toBeVisible();
|
||||
});
|
||||
|
||||
expect(telemetryTrackSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -54,10 +54,37 @@ export function useToast() {
|
|||
}
|
||||
|
||||
if (params.type === 'error' && track) {
|
||||
// Extract string message for telemetry - don't send VNode objects as they have circular refs
|
||||
let messageForTelemetry: string;
|
||||
if (typeof params.message === 'string') {
|
||||
messageForTelemetry = params.message;
|
||||
} else if (
|
||||
params.message &&
|
||||
typeof params.message === 'object' &&
|
||||
'props' in params.message &&
|
||||
params.message.props
|
||||
) {
|
||||
// Extract error message from VNode props (e.g., NodeExecutionErrorMessage component)
|
||||
const props = params.message.props;
|
||||
const hasErrorMessage =
|
||||
typeof props === 'object' && props !== null && 'errorMessage' in props;
|
||||
const hasMessage = typeof props === 'object' && props !== null && 'message' in props;
|
||||
|
||||
if (hasErrorMessage) {
|
||||
messageForTelemetry = String(props.errorMessage);
|
||||
} else if (hasMessage) {
|
||||
messageForTelemetry = String(props.message);
|
||||
} else {
|
||||
messageForTelemetry = 'Unknown error';
|
||||
}
|
||||
} else {
|
||||
messageForTelemetry = 'Unknown error';
|
||||
}
|
||||
|
||||
telemetry.track('Instance FE emitted error', {
|
||||
error_title: params.title,
|
||||
error_message: params.message,
|
||||
caused_by_credential: causedByCredential(params.message as string),
|
||||
error_message: messageForTelemetry,
|
||||
caused_by_credential: causedByCredential(messageForTelemetry),
|
||||
workflow_id: workflowsStore.workflowId,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user