From 5301323906663a64a3042bd2f8946e5f1e3f6293 Mon Sep 17 00:00:00 2001 From: Giulio Andreini Date: Thu, 7 Mar 2024 17:08:01 +0100 Subject: [PATCH] feat(editor): Improve errors in output panel (#8644) Co-authored-by: Michael Kret --- .../nodes/vendors/OpenAi/actions/router.ts | 16 +- packages/cli/src/services/frontend.service.ts | 3 + packages/editor-ui/src/Interface.ts | 2 + .../__tests__/server/endpoints/settings.ts | 1 + packages/editor-ui/src/__tests__/utils.ts | 1 + .../src/components/Error/NodeErrorView.vue | 612 +++++++++++++----- packages/editor-ui/src/components/RunData.vue | 2 +- .../src/plugins/i18n/locales/en.json | 16 + .../editor-ui/src/stores/n8nRoot.store.ts | 4 + .../editor-ui/src/stores/settings.store.ts | 1 + .../v2/actions/base/getSchema.operation.ts | 3 + .../v2/actions/record/create.operation.ts | 4 +- .../actions/record/deleteRecord.operation.ts | 2 +- .../v2/actions/record/get.operation.ts | 2 +- .../v2/actions/record/update.operation.ts | 2 +- .../v2/actions/record/upsert.operation.ts | 4 +- .../nodes/Airtable/v2/helpers/utils.ts | 8 +- .../nodes-base/nodes/Cal/GenericFunctions.ts | 1 + packages/nodes-base/nodes/Code/Code.node.ts | 12 +- .../nodes/Filter/V2/FilterV2.node.ts | 2 + .../nodes/Google/Drive/v2/transport/index.ts | 24 +- .../nodes/Google/Sheet/v2/actions/router.ts | 17 +- .../v2/actions/sheet/remove.operation.ts | 2 +- .../Google/Sheet/v2/helpers/GoogleSheet.ts | 7 +- .../nodes/Google/Sheet/v2/transport/index.ts | 15 +- .../HttpRequest/V3/HttpRequestV3.node.ts | 11 +- .../nodes/Hubspot/V2/HubspotV2.node.ts | 25 +- packages/nodes-base/nodes/If/V2/IfV2.node.ts | 2 + .../nodes/Switch/V3/SwitchV3.node.ts | 3 + .../nodes/UProc/GenericFunctions.ts | 1 + packages/workflow/src/Constants.ts | 5 + packages/workflow/src/Interfaces.ts | 1 + packages/workflow/src/RoutingNode.ts | 10 +- .../src/errors/abstract/node.error.ts | 32 +- .../workflow/src/errors/node-api.error.ts | 70 +- .../src/errors/node-operation.error.ts | 8 +- packages/workflow/test/NodeErrors.test.ts | 98 ++- .../workflow/test/errors/node.error.test.ts | 2 +- 38 files changed, 758 insertions(+), 273 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/router.ts b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/router.ts index ebd9fba2a4e71..b0ce414ce89c7 100644 --- a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/router.ts +++ b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/router.ts @@ -1,4 +1,9 @@ -import { NodeOperationError, type IExecuteFunctions, type INodeExecutionData } from 'n8n-workflow'; +import { + NodeOperationError, + type IExecuteFunctions, + type INodeExecutionData, + NodeApiError, +} from 'n8n-workflow'; import * as assistant from './assistant'; import * as audio from './audio'; @@ -54,6 +59,15 @@ export async function router(this: IExecuteFunctions) { returnData.push({ json: { error: error.message }, pairedItem: { item: i } }); continue; } + + if (error instanceof NodeApiError) { + error.context = { + itemIndex: i, + }; + + throw error; + } + throw new NodeOperationError(this.getNode(), error, { itemIndex: i, description: error.description, diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index d003c7312f1f7..3083e01227424 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -98,6 +98,7 @@ export class FrontendService { timezone: config.getEnv('generic.timezone'), urlBaseWebhook: this.urlService.getWebhookBaseUrl(), urlBaseEditor: instanceBaseUrl, + binaryDataMode: config.getEnv('binaryDataManager.mode'), versionCli: '', releaseChannel: config.getEnv('generic.releaseChannel'), oauthCallbackUrls: { @@ -308,6 +309,8 @@ export class FrontendService { this.settings.executionMode = config.getEnv('executions.mode'); + this.settings.binaryDataMode = config.getEnv('binaryDataManager.mode'); + return this.settings; } diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 58d1b8d914c75..545a7bd202b64 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -1106,6 +1106,7 @@ export interface RootState { urlBaseEditor: string; instanceId: string; isNpmAvailable: boolean; + binaryDataMode: string; } export interface NodeMetadataMap { @@ -1154,6 +1155,7 @@ export interface IRootState { nodeMetadata: NodeMetadataMap; isNpmAvailable: boolean; subworkflowExecutionError: Error | null; + binaryDataMode: string; } export interface CommunityPackageMap { diff --git a/packages/editor-ui/src/__tests__/server/endpoints/settings.ts b/packages/editor-ui/src/__tests__/server/endpoints/settings.ts index 3fa6cc8e67193..d27dc14dbd320 100644 --- a/packages/editor-ui/src/__tests__/server/endpoints/settings.ts +++ b/packages/editor-ui/src/__tests__/server/endpoints/settings.ts @@ -92,6 +92,7 @@ const defaultSettings: IN8nUISettings = { banners: { dismissed: [], }, + binaryDataMode: 'default', }; export function routesForSettings(server: Server) { diff --git a/packages/editor-ui/src/__tests__/utils.ts b/packages/editor-ui/src/__tests__/utils.ts index eec11ca8a45e6..8168b94ae0523 100644 --- a/packages/editor-ui/src/__tests__/utils.ts +++ b/packages/editor-ui/src/__tests__/utils.ts @@ -144,4 +144,5 @@ export const SETTINGS_STORE_DEFAULT_STATE: ISettingsState = { saveDataErrorExecution: 'all', saveDataSuccessExecution: 'all', saveManualExecutions: false, + binaryDataMode: 'default', }; diff --git a/packages/editor-ui/src/components/Error/NodeErrorView.vue b/packages/editor-ui/src/components/Error/NodeErrorView.vue index 85323c6120a25..f7ced1989df0a 100644 --- a/packages/editor-ui/src/components/Error/NodeErrorView.vue +++ b/packages/editor-ui/src/components/Error/NodeErrorView.vue @@ -1,126 +1,216 @@ diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue index 8082dac5d8743..c5dbab8f524df 100644 --- a/packages/editor-ui/src/components/RunData.vue +++ b/packages/editor-ui/src/components/RunData.vue @@ -178,7 +178,7 @@ ((dataCount > 0 && maxRunIndex === 0) || search) && !isArtificialRecoveredEventItem " - v-show="!editMode.enabled" + v-show="!editMode.enabled && !hasRunError" :class="$style.itemsCount" data-test-id="ndv-items-count" > diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index 44261364ac922..ce0d01074685b 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -997,11 +997,27 @@ "nodeCredentials.updateCredential": "Update Credential", "nodeErrorView.cause": "Cause", "nodeErrorView.copyToClipboard": "Copy to Clipboard", + "nodeErrorView.copyToClipboard.tooltip": "Copy error details for debugging. Copied data may contain sensitive information. Proceed with caution when sharing.", "nodeErrorView.dataBelowMayContain": "Data below may contain sensitive information. Proceed with caution when sharing.", "nodeErrorView.details": "Details", + "nodeErrorView.details.from": "From {node}", + "nodeErrorView.details.rawMessages": "Full message", + "nodeErrorView.details.errorData": "Error data", + "nodeErrorView.details.errorExtra": "Error extra", + "nodeErrorView.details.request": "Request", + "nodeErrorView.details.title": "Error details", + "nodeErrorView.details.message": "Error message", + "nodeErrorView.details.info": "Other info", + "nodeErrorView.details.nodeVersion": "Node version", + "nodeErrorView.details.nodeType": "Node type", + "nodeErrorView.details.n8nVersion": "n8n version", + "nodeErrorView.details.errorCause": "Error cause", + "nodeErrorView.details.causeDetailed": "Cause detailed", + "nodeErrorView.details.stackTrace": "Stack trace", "nodeErrorView.error": "ERROR", "nodeErrorView.errorSubNode": "Error in sub-node ‘{node}’", "nodeErrorView.httpCode": "HTTP Code", + "nodeErrorView.errorCode": "Error code", "nodeErrorView.inParameter": "In or underneath Parameter", "nodeErrorView.itemIndex": "Item Index", "nodeErrorView.runIndex": "Run Index", diff --git a/packages/editor-ui/src/stores/n8nRoot.store.ts b/packages/editor-ui/src/stores/n8nRoot.store.ts index 5099da91b98a5..34487bae3498d 100644 --- a/packages/editor-ui/src/stores/n8nRoot.store.ts +++ b/packages/editor-ui/src/stores/n8nRoot.store.ts @@ -30,6 +30,7 @@ export const useRootStore = defineStore(STORES.ROOT, { urlBaseEditor: 'http://localhost:5678', isNpmAvailable: false, instanceId: '', + binaryDataMode: 'default', }), getters: { getBaseUrl(): string { @@ -128,5 +129,8 @@ export const useRootStore = defineStore(STORES.ROOT, { setIsNpmAvailable(isNpmAvailable: boolean): void { this.isNpmAvailable = isNpmAvailable; }, + setBinaryDataMode(binaryDataMode: string): void { + this.binaryDataMode = binaryDataMode; + }, }, }); diff --git a/packages/editor-ui/src/stores/settings.store.ts b/packages/editor-ui/src/stores/settings.store.ts index 42418900cc6f2..645e1f0c6a86b 100644 --- a/packages/editor-ui/src/stores/settings.store.ts +++ b/packages/editor-ui/src/stores/settings.store.ts @@ -280,6 +280,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, { rootStore.setN8nMetadata(settings.n8nMetadata || {}); rootStore.setDefaultLocale(settings.defaultLocale); rootStore.setIsNpmAvailable(settings.isNpmAvailable); + rootStore.setBinaryDataMode(settings.binaryDataMode); useVersionsStore().setVersionNotificationSettings(settings.versionNotifications); }, diff --git a/packages/nodes-base/nodes/Airtable/v2/actions/base/getSchema.operation.ts b/packages/nodes-base/nodes/Airtable/v2/actions/base/getSchema.operation.ts index d4ec44f614d35..aa791f7b64877 100644 --- a/packages/nodes-base/nodes/Airtable/v2/actions/base/getSchema.operation.ts +++ b/packages/nodes-base/nodes/Airtable/v2/actions/base/getSchema.operation.ts @@ -3,10 +3,12 @@ import type { INodeExecutionData, INodeProperties, IExecuteFunctions, + NodeApiError, } from 'n8n-workflow'; import { updateDisplayOptions, wrapData } from '../../../../../utils/utilities'; import { apiRequest } from '../../transport'; import { baseRLC } from '../common.descriptions'; +import { processAirtableError } from '../../helpers/utils'; const properties: INodeProperties[] = [ { @@ -45,6 +47,7 @@ export async function execute( returnData.push(...executionData); } catch (error) { + error = processAirtableError(error as NodeApiError, undefined, i); if (this.continueOnFail()) { returnData.push({ json: { error: error.message } }); continue; diff --git a/packages/nodes-base/nodes/Airtable/v2/actions/record/create.operation.ts b/packages/nodes-base/nodes/Airtable/v2/actions/record/create.operation.ts index 801e5bbfeccef..758c3b7a2ffc1 100644 --- a/packages/nodes-base/nodes/Airtable/v2/actions/record/create.operation.ts +++ b/packages/nodes-base/nodes/Airtable/v2/actions/record/create.operation.ts @@ -3,11 +3,12 @@ import type { INodeExecutionData, INodeProperties, IExecuteFunctions, + NodeApiError, } from 'n8n-workflow'; import { updateDisplayOptions, wrapData } from '../../../../../utils/utilities'; import { apiRequest } from '../../transport'; import { insertUpdateOptions } from '../common.descriptions'; -import { removeIgnored } from '../../helpers/utils'; +import { processAirtableError, removeIgnored } from '../../helpers/utils'; const properties: INodeProperties[] = [ { @@ -85,6 +86,7 @@ export async function execute( returnData.push(...executionData); } catch (error) { + error = processAirtableError(error as NodeApiError, undefined, i); if (this.continueOnFail()) { returnData.push({ json: { message: error.message, error } }); continue; diff --git a/packages/nodes-base/nodes/Airtable/v2/actions/record/deleteRecord.operation.ts b/packages/nodes-base/nodes/Airtable/v2/actions/record/deleteRecord.operation.ts index af934a8bf0d9b..cbbf12ba3c5cc 100644 --- a/packages/nodes-base/nodes/Airtable/v2/actions/record/deleteRecord.operation.ts +++ b/packages/nodes-base/nodes/Airtable/v2/actions/record/deleteRecord.operation.ts @@ -54,7 +54,7 @@ export async function execute( returnData.push(...executionData); } catch (error) { - error = processAirtableError(error as NodeApiError, id); + error = processAirtableError(error as NodeApiError, id, i); if (this.continueOnFail()) { returnData.push({ json: { error: error.message } }); continue; diff --git a/packages/nodes-base/nodes/Airtable/v2/actions/record/get.operation.ts b/packages/nodes-base/nodes/Airtable/v2/actions/record/get.operation.ts index 88767da22f4c9..9742382cddad7 100644 --- a/packages/nodes-base/nodes/Airtable/v2/actions/record/get.operation.ts +++ b/packages/nodes-base/nodes/Airtable/v2/actions/record/get.operation.ts @@ -90,7 +90,7 @@ export async function execute( returnData.push(...executionData); } catch (error) { - error = processAirtableError(error as NodeApiError, id); + error = processAirtableError(error as NodeApiError, id, i); if (this.continueOnFail()) { returnData.push({ json: { error: error.message } }); continue; diff --git a/packages/nodes-base/nodes/Airtable/v2/actions/record/update.operation.ts b/packages/nodes-base/nodes/Airtable/v2/actions/record/update.operation.ts index 7c3f431cfe53d..d2c4172dd53d1 100644 --- a/packages/nodes-base/nodes/Airtable/v2/actions/record/update.operation.ts +++ b/packages/nodes-base/nodes/Airtable/v2/actions/record/update.operation.ts @@ -137,7 +137,7 @@ export async function execute( returnData.push(...executionData); } catch (error) { - error = processAirtableError(error as NodeApiError, recordId); + error = processAirtableError(error as NodeApiError, recordId, i); if (this.continueOnFail()) { returnData.push({ json: { message: error.message, error } }); continue; diff --git a/packages/nodes-base/nodes/Airtable/v2/actions/record/upsert.operation.ts b/packages/nodes-base/nodes/Airtable/v2/actions/record/upsert.operation.ts index b91cfa03372a1..e6266480cee97 100644 --- a/packages/nodes-base/nodes/Airtable/v2/actions/record/upsert.operation.ts +++ b/packages/nodes-base/nodes/Airtable/v2/actions/record/upsert.operation.ts @@ -3,10 +3,11 @@ import type { INodeExecutionData, INodeProperties, IExecuteFunctions, + NodeApiError, } from 'n8n-workflow'; import { updateDisplayOptions, wrapData } from '../../../../../utils/utilities'; import { apiRequest, apiRequestAllItems, batchUpdate } from '../../transport'; -import { removeIgnored } from '../../helpers/utils'; +import { processAirtableError, removeIgnored } from '../../helpers/utils'; import type { UpdateRecord } from '../../helpers/interfaces'; import { insertUpdateOptions } from '../common.descriptions'; @@ -146,6 +147,7 @@ export async function execute( returnData.push(...executionData); } catch (error) { + error = processAirtableError(error as NodeApiError, undefined, i); if (this.continueOnFail()) { returnData.push({ json: { message: error.message, error } }); continue; diff --git a/packages/nodes-base/nodes/Airtable/v2/helpers/utils.ts b/packages/nodes-base/nodes/Airtable/v2/helpers/utils.ts index ff1a186248746..c8f3320c2ac0c 100644 --- a/packages/nodes-base/nodes/Airtable/v2/helpers/utils.ts +++ b/packages/nodes-base/nodes/Airtable/v2/helpers/utils.ts @@ -1,5 +1,6 @@ import { ApplicationError, type IDataObject, type NodeApiError } from 'n8n-workflow'; import type { UpdateRecord } from './interfaces'; +import set from 'lodash/set'; export function removeIgnored(data: IDataObject, ignore: string | string[]) { if (ignore) { @@ -66,13 +67,18 @@ export function findMatches( } } -export function processAirtableError(error: NodeApiError, id?: string) { +export function processAirtableError(error: NodeApiError, id?: string, itemIndex?: number) { if (error.description === 'NOT_FOUND' && id) { error.description = `${id} is not a valid Record ID`; } if (error.description?.includes('You must provide an array of up to 10 record objects') && id) { error.description = `${id} is not a valid Record ID`; } + + if (itemIndex !== undefined) { + set(error, 'context.itemIndex', itemIndex); + } + return error; } diff --git a/packages/nodes-base/nodes/Cal/GenericFunctions.ts b/packages/nodes-base/nodes/Cal/GenericFunctions.ts index e95eb42cfedfa..d1aecef3640ed 100644 --- a/packages/nodes-base/nodes/Cal/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Cal/GenericFunctions.ts @@ -37,6 +37,7 @@ export async function calApiRequest( try { return await this.helpers.httpRequestWithAuthentication.call(this, 'calApi', options); } catch (error) { + if (error instanceof NodeApiError) throw error; throw new NodeApiError(this.getNode(), error as JsonObject); } } diff --git a/packages/nodes-base/nodes/Code/Code.node.ts b/packages/nodes-base/nodes/Code/Code.node.ts index 2ddc46db89f2d..ff52f409f75bd 100644 --- a/packages/nodes-base/nodes/Code/Code.node.ts +++ b/packages/nodes-base/nodes/Code/Code.node.ts @@ -13,6 +13,8 @@ import { PythonSandbox } from './PythonSandbox'; import { getSandboxContext } from './Sandbox'; import { standardizeOutput } from './utils'; +import set from 'lodash/set'; + const { CODE_ENABLE_STDOUT } = process.env; export class Code implements INodeType { @@ -133,7 +135,10 @@ export class Code implements INodeType { try { items = (await sandbox.runCodeAllItems()) as INodeExecutionData[]; } catch (error) { - if (!this.continueOnFail()) throw error; + if (!this.continueOnFail()) { + set(error, 'node', node); + throw error; + } items = [{ json: { error: error.message } }]; } @@ -158,7 +163,10 @@ export class Code implements INodeType { try { result = await sandbox.runCodeEachItem(); } catch (error) { - if (!this.continueOnFail()) throw error; + if (!this.continueOnFail()) { + set(error, 'node', node); + throw error; + } returnData.push({ json: { error: error.message }, pairedItem: { diff --git a/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts b/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts index c74498e5053a8..5163df2c5cbef 100644 --- a/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts +++ b/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts @@ -86,6 +86,8 @@ export class FilterV2 implements INodeType { "Try to change the operator, switch ON the option 'Less Strict Type Validation', or change the type with an expression", ); } + set(error, 'context.itemIndex', itemIndex); + set(error, 'node', this.getNode()); throw error; } diff --git a/packages/nodes-base/nodes/Google/Drive/v2/transport/index.ts b/packages/nodes-base/nodes/Google/Drive/v2/transport/index.ts index 2af1bdb3be7bf..b0d984f529b38 100644 --- a/packages/nodes-base/nodes/Google/Drive/v2/transport/index.ts +++ b/packages/nodes-base/nodes/Google/Drive/v2/transport/index.ts @@ -57,31 +57,13 @@ export async function googleApiRequest( ); } } catch (error) { + if (error instanceof NodeApiError) throw error; + if (error.code === 'ERR_OSSL_PEM_NO_START_LINE') { error.statusCode = '401'; } - const apiError = new NodeApiError( - this.getNode(), - { - reason: error.error, - } as JsonObject, - { httpCode: String(error.statusCode) }, - ); - - if ( - apiError.message && - apiError.description && - (apiError.message.toLowerCase().includes('bad request') || - apiError.message.toLowerCase().includes('forbidden') || - apiError.message.toUpperCase().includes('UNKNOWN ERROR')) - ) { - const message = apiError.message; - apiError.message = apiError.description; - apiError.description = message; - } - - throw apiError; + throw new NodeApiError(this.getNode(), error as JsonObject); } } diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts b/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts index 591567c6f8ca8..5159e9b76e565 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts @@ -1,4 +1,4 @@ -import type { IExecuteFunctions, IDataObject, INodeExecutionData } from 'n8n-workflow'; +import { type IExecuteFunctions, type IDataObject, type INodeExecutionData } from 'n8n-workflow'; import { GoogleSheet } from '../helpers/GoogleSheet'; import { getSpreadsheetId } from '../helpers/GoogleSheets.utils'; import type { GoogleSheets, ResourceLocator } from '../helpers/GoogleSheets.types'; @@ -72,20 +72,11 @@ export async function router(this: IExecuteFunctions): Promise { const returnData: INodeExecutionData[] = []; diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheet.ts b/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheet.ts index a7d532b1144fc..6305577a94324 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheet.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheet.ts @@ -139,11 +139,8 @@ export class GoogleSheet { }); if (!foundItem?.properties?.title) { - throw new NodeOperationError( - node, - `Sheet with ${mode === 'name' ? 'name' : 'ID'} ${value} not found`, - { level: 'warning' }, - ); + const error = new Error(`Sheet with ${mode === 'name' ? 'name' : 'ID'} ${value} not found`); + throw new NodeOperationError(node, error, { level: 'warning' }); } return foundItem.properties; diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/transport/index.ts b/packages/nodes-base/nodes/Google/Sheet/v2/transport/index.ts index 7a52b64faa9ec..715ef3818b03b 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/transport/index.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/transport/index.ts @@ -9,6 +9,7 @@ import type { } from 'n8n-workflow'; import { NodeApiError } from 'n8n-workflow'; import { getGoogleAccessToken } from '../../../GenericFunctions'; +import set from 'lodash/set'; export async function apiRequest( this: IExecuteFunctions | ILoadOptionsFunctions | IPollFunctions, @@ -62,11 +63,15 @@ export async function apiRequest( error.statusCode = '401'; } - if (error.message.includes('PERMISSION_DENIED')) { - const message = `Missing permissions for Google Sheet, ${error.message}}`; - const details = error.description ? ` Details of the error: ${error.description}.` : ''; - const description = `Please check that the account you're using has the right permissions. (If you're trying to modify the sheet, you'll need edit access.)${details}`; - throw new NodeApiError(this.getNode(), error as JsonObject, { message, description }); + if (error instanceof NodeApiError) { + if (error.message.includes('PERMISSION_DENIED')) { + const details = error.description ? ` Details of the error: ${error.description}.` : ''; + const description = `Please check that the account you're using has the right permissions. (If you're trying to modify the sheet, you'll need edit access.)${details}`; + + set(error, 'description', description); + } + + throw error; } throw new NodeApiError(this.getNode(), error as JsonObject); diff --git a/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts b/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts index 1dd00cd7ea386..b0c2ad685b1b4 100644 --- a/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts +++ b/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts @@ -34,6 +34,7 @@ import { sanitizeUiMessage, } from '../GenericFunctions'; import { keysToLowercase } from '@utils/utilities'; +import set from 'lodash/set'; function toText(data: T) { if (typeof data === 'object' && data !== null) { @@ -1255,6 +1256,7 @@ export class HttpRequestV3 implements INodeType { requestInterval: number; }; + const sanitazedRequests: IDataObject[] = []; for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { if (authentication === 'genericCredentialType') { genericCredentialType = this.getNodeParameter('genericAuthType', 0) as string; @@ -1627,8 +1629,11 @@ export class HttpRequestV3 implements INodeType { 'application/json,text/html,application/xhtml+xml,application/xml,text/*;q=0.9, image/*;q=0.8, */*;q=0.7'; } } + try { - this.sendMessageToUI(sanitizeUiMessage(requestOptions, authDataKeys)); + const sanitazedRequestOptions = sanitizeUiMessage(requestOptions, authDataKeys); + this.sendMessageToUI(sanitazedRequestOptions); + sanitazedRequests.push(sanitazedRequestOptions); } catch (e) {} if (pagination && pagination.paginationMode !== 'off') { @@ -1770,7 +1775,9 @@ export class HttpRequestV3 implements INodeType { if (autoDetectResponseFormat && responseData.reason.error instanceof Buffer) { responseData.reason.error = Buffer.from(responseData.reason.error as Buffer).toString(); } - throw new NodeApiError(this.getNode(), responseData as JsonObject, { itemIndex }); + const error = new NodeApiError(this.getNode(), responseData as JsonObject, { itemIndex }); + set(error, 'context.request', sanitazedRequests[itemIndex]); + throw error; } else { removeCircularRefs(responseData.reason as JsonObject); // Return the actual reason as error diff --git a/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts b/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts index bdb6e08d4d4c4..4e31b8e7f3f9b 100644 --- a/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts +++ b/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts @@ -15,7 +15,7 @@ import type { INodeTypeDescription, JsonObject, } from 'n8n-workflow'; -import { NodeOperationError } from 'n8n-workflow'; +import { NodeApiError, NodeOperationError } from 'n8n-workflow'; import { snakeCase } from 'change-case'; import { generatePairedItemData } from '../../../utils/utilities'; @@ -47,6 +47,8 @@ import type { IForm } from './FormInterface'; import type { IAssociation, IDeal } from './DealInterface'; +import set from 'lodash/set'; + export class HubspotV2 implements INodeType { description: INodeTypeDescription; @@ -3058,21 +3060,17 @@ export class HubspotV2 implements INodeType { error.cause.error?.validationResults && error.cause.error.validationResults[0].error === 'INVALID_EMAIL' ) { - throw new NodeOperationError( - this.getNode(), - error.cause.error.validationResults[0].message as string, - ); + const message = error.cause.error.validationResults[0].message as string; + set(error, 'message', message); } if (error.cause.error?.message !== 'The resource you are requesting could not be found') { if (error.httpCode === '404' && error.description === 'resource not found') { - throw new NodeOperationError( - this.getNode(), - `${error.node.parameters.resource} #${ - error.node.parameters[`${error.node.parameters.resource}Id`].value - } could not be found. Check your ${error.node.parameters.resource} ID is correct`, - ); + const message = `${error.node.parameters.resource} #${ + error.node.parameters[`${error.node.parameters.resource}Id`].value + } could not be found. Check your ${error.node.parameters.resource} ID is correct`; + + set(error, 'message', message); } - throw new NodeOperationError(this.getNode(), error as string); } if (this.continueOnFail()) { returnData.push({ @@ -3081,6 +3079,9 @@ export class HubspotV2 implements INodeType { }); continue; } + if (error instanceof NodeApiError) { + set(error, 'context.itemIndex', i); + } throw error; } } diff --git a/packages/nodes-base/nodes/If/V2/IfV2.node.ts b/packages/nodes-base/nodes/If/V2/IfV2.node.ts index 3cf0df38680c6..fb4fa7745426e 100644 --- a/packages/nodes-base/nodes/If/V2/IfV2.node.ts +++ b/packages/nodes-base/nodes/If/V2/IfV2.node.ts @@ -86,6 +86,8 @@ export class IfV2 implements INodeType { "Try to change the operator, switch ON the option 'Less Strict Type Validation', or change the type with an expression", ); } + set(error, 'context.itemIndex', itemIndex); + set(error, 'node', this.getNode()); throw error; } diff --git a/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts b/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts index eceefdd79747b..cc0eb852ae3b3 100644 --- a/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts +++ b/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts @@ -11,6 +11,7 @@ import type { } from 'n8n-workflow'; import { NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import { capitalize } from '@utils/utilities'; +import set from 'lodash/set'; const configuredOutputs = (parameters: INodeParameters) => { const mode = parameters.mode as string; @@ -351,6 +352,8 @@ export class SwitchV3 implements INodeType { error.description = "Try to change the operator, switch ON the option 'Less Strict Type Validation', or change the type with an expression"; } + set(error, 'context.itemIndex', itemIndex); + set(error, 'node', this.getNode()); throw error; } diff --git a/packages/nodes-base/nodes/UProc/GenericFunctions.ts b/packages/nodes-base/nodes/UProc/GenericFunctions.ts index b5cb760b9d5bb..bad8de0251cd1 100644 --- a/packages/nodes-base/nodes/UProc/GenericFunctions.ts +++ b/packages/nodes-base/nodes/UProc/GenericFunctions.ts @@ -28,6 +28,7 @@ export async function uprocApiRequest( try { return await this.helpers.httpRequestWithAuthentication.call(this, 'uprocApi', options); } catch (error) { + if (error instanceof NodeApiError) throw error; throw new NodeApiError(this.getNode(), error as JsonObject); } } diff --git a/packages/workflow/src/Constants.ts b/packages/workflow/src/Constants.ts index 12f33a5d11306..4ab3e315a0d1a 100644 --- a/packages/workflow/src/Constants.ts +++ b/packages/workflow/src/Constants.ts @@ -12,6 +12,11 @@ export const CREDENTIAL_EMPTY_VALUE = export const FORM_TRIGGER_PATH_IDENTIFIER = 'n8n-form'; +export const UNKNOWN_ERROR_MESSAGE = 'There was an unknown issue while executing the node'; +export const UNKNOWN_ERROR_DESCRIPTION = + 'Double-check the node configuration and the service it connects to. Check the error details below and refer to the n8n documentation to troubleshoot the issue.'; +export const UNKNOWN_ERROR_MESSAGE_CRED = 'UNKNOWN ERROR'; + //n8n-nodes-base export const STICKY_NODE_TYPE = 'n8n-nodes-base.stickyNote'; export const NO_OP_NODE_TYPE = 'n8n-nodes-base.noOp'; diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 54bad71b4b42c..297ee9fa19fb5 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2474,6 +2474,7 @@ export interface IN8nUISettings { urlBaseWebhook: string; urlBaseEditor: string; versionCli: string; + binaryDataMode: string; releaseChannel: 'stable' | 'beta' | 'nightly' | 'dev'; n8nMetadata?: { userId?: string; diff --git a/packages/workflow/src/RoutingNode.ts b/packages/workflow/src/RoutingNode.ts index 187861f720bc8..1a3490ce9a756 100644 --- a/packages/workflow/src/RoutingNode.ts +++ b/packages/workflow/src/RoutingNode.ts @@ -227,15 +227,19 @@ export class RoutingNode { continue; } + if (error instanceof NodeApiError) { + set(error, 'context.itemIndex', i); + set(error, 'context.runIndex', runIndex); + throw error; + } + interface AxiosError extends NodeError { isAxiosError: boolean; description: string | undefined; response?: { status: number }; } - let routingError = error as AxiosError; - - if (error instanceof NodeApiError && error.cause) routingError = error.cause as AxiosError; + const routingError = error as AxiosError; throw new NodeApiError(this.node, error as JsonObject, { runIndex, diff --git a/packages/workflow/src/errors/abstract/node.error.ts b/packages/workflow/src/errors/abstract/node.error.ts index d724991acf3e9..060751cd86c5e 100644 --- a/packages/workflow/src/errors/abstract/node.error.ts +++ b/packages/workflow/src/errors/abstract/node.error.ts @@ -35,6 +35,8 @@ const COMMON_ERRORS: IDataObject = { * a value recursively inside an error object. */ export abstract class NodeError extends ExecutionBaseError { + messages: string[] = []; + constructor( readonly node: INode, error: Error | JsonObject, @@ -123,52 +125,56 @@ export abstract class NodeError extends ExecutionBaseError { return null; } + /** + * Preserve the original error message before setting the new one + */ + protected addToMessages(message: string): void { + if (message && !this.messages.includes(message)) { + this.messages.push(message); + } + } + /** * Set descriptive error message if code is provided or if message contains any of the common errors, * update description to include original message plus the description */ protected setDescriptiveErrorMessage( message: string, - description: string | undefined | null, + messages: string[], code?: string | null, messageMapping?: { [key: string]: string }, - ) { + ): [string, string[]] { let newMessage = message; - let newDescription = description as string; if (messageMapping) { for (const [mapKey, mapMessage] of Object.entries(messageMapping)) { if ((message || '').toUpperCase().includes(mapKey.toUpperCase())) { newMessage = mapMessage; - newDescription = this.updateDescription(message, description); + messages.push(message); break; } } if (newMessage !== message) { - return [newMessage, newDescription]; + return [newMessage, messages]; } } // if code is provided and it is in the list of common errors set the message and return early if (code && COMMON_ERRORS[code.toUpperCase()]) { newMessage = COMMON_ERRORS[code] as string; - newDescription = this.updateDescription(message, description); - return [newMessage, newDescription]; + messages.push(message); + return [newMessage, messages]; } // check if message contains any of the common errors and set the message and description for (const [errorCode, errorDescriptiveMessage] of Object.entries(COMMON_ERRORS)) { if ((message || '').toUpperCase().includes(errorCode.toUpperCase())) { newMessage = errorDescriptiveMessage as string; - newDescription = this.updateDescription(message, description); + messages.push(message); break; } } - return [newMessage, newDescription]; - } - - protected updateDescription(message: string, description: string | undefined | null) { - return `${message}${description ? ` - ${description}` : ''}`; + return [newMessage, messages]; } } diff --git a/packages/workflow/src/errors/node-api.error.ts b/packages/workflow/src/errors/node-api.error.ts index 0048c9c786f66..17fd3871e5d16 100644 --- a/packages/workflow/src/errors/node-api.error.ts +++ b/packages/workflow/src/errors/node-api.error.ts @@ -15,7 +15,12 @@ import { NodeError } from './abstract/node.error'; import { removeCircularRefs } from '../utils'; import type { ReportingOptions } from './application.error'; import { AxiosError } from 'axios'; -import { NO_OP_NODE_TYPE } from '../Constants'; +import { + NO_OP_NODE_TYPE, + UNKNOWN_ERROR_DESCRIPTION, + UNKNOWN_ERROR_MESSAGE, + UNKNOWN_ERROR_MESSAGE_CRED, +} from '../Constants'; export interface NodeOperationErrorOptions { message?: string; @@ -103,9 +108,6 @@ const STATUS_CODE_MESSAGES: IStatusCodeMessages = { '504': 'Gateway timed out - perhaps try again later?', }; -const UNKNOWN_ERROR_MESSAGE = 'UNKNOWN ERROR - check the detailed error for more information'; -const UNKNOWN_ERROR_MESSAGE_CRED = 'UNKNOWN ERROR'; - /** * Class for instantiating an error in an API response, e.g. a 404 Not Found response, * with an HTTP error code, an error message and a description. @@ -130,6 +132,8 @@ export class NodeApiError extends NodeError { ) { super(node, errorResponse); + this.addToMessages(errorResponse.message as string); + if (!httpCode && errorResponse instanceof AxiosError) { httpCode = errorResponse.response?.status?.toString(); } @@ -176,6 +180,8 @@ export class NodeApiError extends NodeError { // set http code of this error if (httpCode) { this.httpCode = httpCode; + } else if (errorResponse.httpCode) { + this.httpCode = errorResponse.httpCode as string; } else { this.httpCode = this.findProperty(errorResponse, ERROR_STATUS_PROPERTIES, ERROR_NESTING_PROPERTIES) ?? null; @@ -187,6 +193,25 @@ export class NodeApiError extends NodeError { this.level = 'warning'; } + if ( + errorResponse?.response && + typeof errorResponse?.response === 'object' && + !Array.isArray(errorResponse.response) && + errorResponse.response.data && + typeof errorResponse.response.data === 'object' && + !Array.isArray(errorResponse.response.data) + ) { + const data = errorResponse.response.data; + + if (data.message) { + description = data.message as string; + } else if (data.error && ((data.error as IDataObject) || {}).message) { + description = (data.error as IDataObject).message as string; + } + + this.context.data = data; + } + // set description of this error if (description) { this.description = description; @@ -204,7 +229,9 @@ export class NodeApiError extends NodeError { } } - // set message if provided or set default message based on http code + // set message if provided + // set default message based on http code + // or use raw error message if (message) { this.message = message; } else { @@ -217,9 +244,9 @@ export class NodeApiError extends NodeError { } // if message contain common error code set descriptive message and update description - [this.message, this.description] = this.setDescriptiveErrorMessage( + [this.message, this.messages] = this.setDescriptiveErrorMessage( this.message, - this.description, + this.messages, // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing this.httpCode || (errorResponse?.code as string) || @@ -259,29 +286,44 @@ export class NodeApiError extends NodeError { if (!this.httpCode) { this.httpCode = null; - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - this.message = this.message || this.description || UNKNOWN_ERROR_MESSAGE; + + if (!this.message) { + if (this.description) { + this.message = this.description; + this.description = undefined; + } else { + this.message = UNKNOWN_ERROR_MESSAGE; + this.description = UNKNOWN_ERROR_DESCRIPTION; + } + } return; } if (STATUS_CODE_MESSAGES[this.httpCode]) { - this.description = this.updateDescription(this.message, this.description); + this.addToMessages(this.message); this.message = STATUS_CODE_MESSAGES[this.httpCode]; return; } switch (this.httpCode.charAt(0)) { case '4': - this.description = this.updateDescription(this.message, this.description); + this.addToMessages(this.message); this.message = STATUS_CODE_MESSAGES['4XX']; break; case '5': - this.description = this.updateDescription(this.message, this.description); + this.addToMessages(this.message); this.message = STATUS_CODE_MESSAGES['5XX']; break; default: - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - this.message = this.message || this.description || UNKNOWN_ERROR_MESSAGE; + if (!this.message) { + if (this.description) { + this.message = this.description; + this.description = undefined; + } else { + this.message = UNKNOWN_ERROR_MESSAGE; + this.description = UNKNOWN_ERROR_DESCRIPTION; + } + } } if (this.node.type === NO_OP_NODE_TYPE && this.message === UNKNOWN_ERROR_MESSAGE) { this.message = `${UNKNOWN_ERROR_MESSAGE_CRED} - ${this.httpCode}`; diff --git a/packages/workflow/src/errors/node-operation.error.ts b/packages/workflow/src/errors/node-operation.error.ts index 107d0e3ccd059..3a41f1903e890 100644 --- a/packages/workflow/src/errors/node-operation.error.ts +++ b/packages/workflow/src/errors/node-operation.error.ts @@ -20,6 +20,10 @@ export class NodeOperationError extends NodeError { } super(node, error); + if (error instanceof NodeError && error?.messages?.length) { + error.messages.forEach((message) => this.addToMessages(message)); + } + if (options.message) this.message = options.message; if (options.level) this.level = options.level; if (options.functionality) this.functionality = options.functionality; @@ -32,9 +36,9 @@ export class NodeOperationError extends NodeError { this.description = undefined; } - [this.message, this.description] = this.setDescriptiveErrorMessage( + [this.message, this.messages] = this.setDescriptiveErrorMessage( this.message, - this.description, + this.messages, undefined, options.messageMapping, ); diff --git a/packages/workflow/test/NodeErrors.test.ts b/packages/workflow/test/NodeErrors.test.ts index 547a8d0533aac..8b9f7fefbbb91 100644 --- a/packages/workflow/test/NodeErrors.test.ts +++ b/packages/workflow/test/NodeErrors.test.ts @@ -1,6 +1,7 @@ import type { INode } from '@/Interfaces'; import { NodeOperationError } from '@/errors'; import { NodeApiError } from '@/errors/node-api.error'; +import { UNKNOWN_ERROR_DESCRIPTION, UNKNOWN_ERROR_MESSAGE } from '../src/Constants'; const node: INode = { id: '1', @@ -17,9 +18,7 @@ describe('NodeErrors tests', () => { it('should return unknown error message', () => { const nodeApiError = new NodeApiError(node, {}); - expect(nodeApiError.message).toEqual( - 'UNKNOWN ERROR - check the detailed error for more information', - ); + expect(nodeApiError.message).toEqual(UNKNOWN_ERROR_MESSAGE); }); it('should return the error message', () => { @@ -110,9 +109,8 @@ describe('NodeErrors tests', () => { expect(nodeOperationError.message).toEqual('The server closed the connection unexpectedly'); - expect(nodeOperationError.description).toEqual( - 'GETADDRINFO test error message - test error description', - ); + //description should not include error message + expect(nodeOperationError.description).toEqual('test error description'); }); it('should remove description if it is equal to message, NodeOperationError', () => { @@ -175,3 +173,91 @@ describe('NodeErrors tests', () => { ); }); }); + +describe('NodeApiError message and description logic', () => { + it('case: customMessage && customDescription, result: message === customMessage; description === customDescription', () => { + const apiError = { message: 'Original message', code: 404 }; + const nodeApiError = new NodeApiError(node, apiError, { + message: 'Custom message', + description: 'Custom description', + }); + + expect(nodeApiError.message).toEqual('Custom message'); + expect(nodeApiError.description).toEqual('Custom description'); + expect(nodeApiError.messages).toContain('Original message'); + }); + + it('case: customMessage && !customDescription && extractedMessage, result: message === customMessage; description === extractedMessage', () => { + const apiError = { + message: 'Original message', + code: 404, + response: { data: { error: { message: 'Extracted message' } } }, + }; + const nodeApiError = new NodeApiError(node, apiError, { + message: 'Custom message', + }); + + expect(nodeApiError.message).toEqual('Custom message'); + expect(nodeApiError.description).toEqual('Extracted message'); + expect(nodeApiError.messages).toContain('Original message'); + }); + + it('case: customMessage && !customDescription && !extractedMessage, result: message === customMessage; !description', () => { + const apiError = { + message: '', + code: 404, + response: { data: { error: { foo: 'Extracted message' } } }, + }; + const nodeApiError = new NodeApiError(node, apiError, { + message: 'Custom message', + }); + + expect(nodeApiError.message).toEqual('Custom message'); + expect(nodeApiError.description).toBeFalsy(); + expect(nodeApiError.messages.length).toBe(0); + }); + + it('case: !customMessage && httpCodeMapping && extractedMessage, result: message === httpCodeMapping; description === extractedMessage', () => { + const apiError = { + message: 'Original message', + code: 404, + response: { data: { error: { message: 'Extracted message' } } }, + }; + const nodeApiError = new NodeApiError(node, apiError); + + expect(nodeApiError.message).toEqual('The resource you are requesting could not be found'); + expect(nodeApiError.description).toEqual('Extracted message'); + expect(nodeApiError.messages).toContain('Original message'); + }); + + it('case: !customMessage && httpCodeMapping && !extractedMessage, result: message === httpCodeMapping; !description', () => { + const apiError = { + message: '', + code: 500, + }; + const nodeApiError = new NodeApiError(node, apiError); + + expect(nodeApiError.message).toEqual('The service was not able to process your request'); + expect(nodeApiError.description).toBeFalsy(); + }); + + it('case: !customMessage && !httpCodeMapping && extractedMessage, result: message === extractedMessage; !description', () => { + const apiError = { + message: '', + code: 300, + response: { data: { error: { message: 'Extracted message' } } }, + }; + const nodeApiError = new NodeApiError(node, apiError); + + expect(nodeApiError.message).toEqual('Extracted message'); + expect(nodeApiError.description).toBeFalsy(); + }); + + it('case: !customMessage && !httpCodeMapping && !extractedMessage, result: message === UNKNOWN_ERROR_MESSAGE; description === UNKNOWN_ERROR_DESCRIPTION', () => { + const apiError = {}; + const nodeApiError = new NodeApiError(node, apiError); + + expect(nodeApiError.message).toEqual(UNKNOWN_ERROR_MESSAGE); + expect(nodeApiError.description).toEqual(UNKNOWN_ERROR_DESCRIPTION); + }); +}); diff --git a/packages/workflow/test/errors/node.error.test.ts b/packages/workflow/test/errors/node.error.test.ts index 4c5529bed53ba..3bf7dc20a6112 100644 --- a/packages/workflow/test/errors/node.error.test.ts +++ b/packages/workflow/test/errors/node.error.test.ts @@ -7,7 +7,7 @@ describe('NodeError', () => { const node = mock(); it('should update re-wrapped error level and message', () => { - const apiError = new NodeApiError(node, mock({ message: 'Some error happened', code: 500 })); + const apiError = new NodeApiError(node, { message: 'Some error happened', code: 500 }); const opsError = new NodeOperationError(node, mock(), { message: 'Some operation failed' }); const wrapped1 = new NodeOperationError(node, apiError); const wrapped2 = new NodeOperationError(node, opsError);