From c0bdf3b7199a35efb614b62d4e5230b5020542cb Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Mon, 5 Aug 2024 16:42:06 +0300 Subject: [PATCH] fix(core): Move error obfuscation to FE (no-changelog) (#10237) --- packages/core/src/WorkflowExecute.ts | 26 ++++++++++++++++--- .../src/components/Error/NodeErrorView.vue | 4 +++ .../src/plugins/i18n/locales/en.json | 1 + .../src/errors/node-operation.error.ts | 7 ++--- .../workflow/test/errors/node.error.test.ts | 8 ++++-- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index ce17eafa9b..303b46b20a 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -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'; @@ -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, diff --git a/packages/editor-ui/src/components/Error/NodeErrorView.vue b/packages/editor-ui/src/components/Error/NodeErrorView.vue index a0d6396df1..6b6238338d 100644 --- a/packages/editor-ui/src/components/Error/NodeErrorView.vue +++ b/packages/editor-ui/src/components/Error/NodeErrorView.vue @@ -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 = diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index 2acf1cfd35..e4b312f77f 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -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", diff --git a/packages/workflow/src/errors/node-operation.error.ts b/packages/workflow/src/errors/node-operation.error.ts index b0167250dc..8f6cd7a028 100644 --- a/packages/workflow/src/errors/node-operation.error.ts +++ b/packages/workflow/src/errors/node-operation.error.ts @@ -2,7 +2,6 @@ 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. @@ -10,6 +9,8 @@ import { OBFUSCATED_ERROR_MESSAGE } from '../Constants'; export class NodeOperationError extends NodeError { type: string | undefined; + obfuscate: boolean = false; + constructor( node: INode, error: Error | string | JsonObject, @@ -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; @@ -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; diff --git a/packages/workflow/test/errors/node.error.test.ts b/packages/workflow/test/errors/node.error.test.ts index 357314c347..4ec8c1d611 100644 --- a/packages/workflow/test/errors/node.error.test.ts +++ b/packages/workflow/test/errors/node.error.test.ts @@ -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(); @@ -22,7 +21,8 @@ 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'); }); @@ -30,6 +30,7 @@ describe('NodeError', () => { 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'); }); @@ -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); }); @@ -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'); }); @@ -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'); });