Skip to content

Commit

Permalink
fix(core): Move error obfuscation to FE (no-changelog) (n8n-io#10237)
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-radency committed Aug 5, 2024
1 parent 06419d9 commit c0bdf3b
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 9 deletions.
26 changes: 22 additions & 4 deletions packages/core/src/WorkflowExecute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
ApplicationError,
NodeExecutionOutput,
sleep,
OBFUSCATED_ERROR_MESSAGE,
ErrorReporterProxy,
} from 'n8n-workflow';
import get from 'lodash/get';
import * as NodeExecuteFunctions from './NodeExecuteFunctions';
Expand Down Expand Up @@ -1318,12 +1318,30 @@ export class WorkflowExecute {
} catch (error) {
this.runExecutionData.resultData.lastNodeExecuted = executionData.node.name;

const message =
error instanceof ApplicationError ? error.message : OBFUSCATED_ERROR_MESSAGE;
let toReport: Error | undefined;
if (error instanceof ApplicationError) {
// Report any unhandled errors that were wrapped in by one of our error classes
if (error.cause instanceof Error) toReport = error.cause;
} else {
// Report any unhandled and non-wrapped errors to Sentry
toReport = error;
// Set obfuscate to true so that the error would be obfuscated in th UI
error.obfuscate = true;
}
if (toReport) {
ErrorReporterProxy.error(toReport, {
extra: {
nodeName: executionNode.name,
nodeType: executionNode.type,
nodeVersion: executionNode.typeVersion,
workflowId: workflow.id,
},
});
}

const e = error as unknown as ExecutionBaseError;

executionError = { ...e, message, stack: e.stack };
executionError = { ...e, message: e.message, stack: e.stack };

Logger.debug(`Running node "${executionNode.name}" finished with error`, {
node: executionNode.name,
Expand Down
4 changes: 4 additions & 0 deletions packages/editor-ui/src/components/Error/NodeErrorView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ function addItemIndexSuffix(message: string): string {
}
function getErrorMessage(): string {
if ('obfuscate' in props.error && props.error.obfuscate === true) {
return i18n.baseText('nodeErrorView.showMessage.obfuscate');
}
let message = '';
const isSubNodeError =
Expand Down
1 change: 1 addition & 0 deletions packages/editor-ui/src/plugins/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,7 @@
"nodeErrorView.itemIndex": "Item Index",
"nodeErrorView.runIndex": "Run Index",
"nodeErrorView.showMessage.title": "Copied to clipboard",
"nodeErrorView.showMessage.obfuscate": "Internal error",
"nodeErrorView.stack": "Stack",
"nodeErrorView.theErrorCauseIsTooLargeToBeDisplayed": "The error cause is too large to be displayed",
"nodeErrorView.time": "Time",
Expand Down
7 changes: 4 additions & 3 deletions packages/workflow/src/errors/node-operation.error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import type { INode, JsonObject } from '@/Interfaces';
import type { NodeOperationErrorOptions } from './node-api.error';
import { NodeError } from './abstract/node.error';
import { ApplicationError } from './application.error';
import { OBFUSCATED_ERROR_MESSAGE } from '../Constants';

/**
* Class for instantiating an operational error, e.g. an invalid credentials error.
*/
export class NodeOperationError extends NodeError {
type: string | undefined;

obfuscate: boolean = false;

constructor(
node: INode,
error: Error | string | JsonObject,
Expand All @@ -22,7 +23,7 @@ export class NodeOperationError extends NodeError {
let obfuscateErrorMessage = false;

if (typeof error === 'string') {
error = new Error(error);
error = new ApplicationError(error);
} else if (!(error instanceof ApplicationError)) {
// this error was no processed by n8n, obfuscate error message
obfuscateErrorMessage = true;
Expand All @@ -37,7 +38,7 @@ export class NodeOperationError extends NodeError {
if (obfuscateErrorMessage && !options.description) {
const originalMessage = typeof error === 'string' ? error : (error.message as string);
this.addToMessages(originalMessage);
this.message = OBFUSCATED_ERROR_MESSAGE;
this.obfuscate = true;
}
if (options.message) this.message = options.message;
if (options.level) this.level = options.level;
Expand Down
8 changes: 6 additions & 2 deletions packages/workflow/test/errors/node.error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { INode } from '@/Interfaces';
import { NodeApiError } from '@/errors/node-api.error';
import { NodeOperationError } from '@/errors/node-operation.error';
import { ApplicationError } from '@/errors/application.error';
import { OBFUSCATED_ERROR_MESSAGE } from '@/Constants';

describe('NodeError', () => {
const node = mock<INode>();
Expand All @@ -22,14 +21,16 @@ describe('NodeError', () => {
const error = new Error('Original error message');
const nodeOpError = new NodeOperationError(node, error);

expect(nodeOpError.message).toBe(OBFUSCATED_ERROR_MESSAGE);
expect(nodeOpError.obfuscate).toBe(true);
expect(nodeOpError.message).toBe('Original error message');
expect(nodeOpError.messages).toContain('Original error message');
});

it('should not obfuscate errors processed by n8n', () => {
const appError = new ApplicationError('Processed error message');
const nodeOpError = new NodeOperationError(node, appError);

expect(nodeOpError.obfuscate).toBe(false);
expect(nodeOpError.message).toBe('Processed error message');
expect(nodeOpError.messages).not.toContain('Processed error message');
});
Expand All @@ -38,6 +39,7 @@ describe('NodeError', () => {
const errorMessage = 'String error message';
const nodeOpError = new NodeOperationError(node, errorMessage);

expect(nodeOpError.obfuscate).toBe(false);
expect(nodeOpError.message).toBe(errorMessage);
expect(nodeOpError.messages).toHaveLength(0);
});
Expand All @@ -47,6 +49,7 @@ describe('NodeError', () => {
const options = { description: 'Error description' };
const nodeOpError = new NodeOperationError(node, error, options);

expect(nodeOpError.obfuscate).toBe(false);
expect(nodeOpError.message).toBe('Initial error message');
});

Expand All @@ -55,6 +58,7 @@ describe('NodeError', () => {
const options = { message: 'Overridden message', description: 'Error description' };
const nodeOpError = new NodeOperationError(node, error, options);

expect(nodeOpError.obfuscate).toBe(false);
expect(nodeOpError.message).toBe('Overridden message');
expect(nodeOpError.description).toBe('Error description');
});
Expand Down

0 comments on commit c0bdf3b

Please sign in to comment.