feat(editor): Improve errors in output panel (#8644)

Co-authored-by: Michael Kret <michael.k@radency.com>

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 <a href="https://docs.n8n.io" target="_blank">n8n documentation</a> 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';

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;

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,

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];
 	}
 }

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}`;

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,
 		);

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);
+	});
+});

packages/workflow/test/errors/node.error.test.ts

@@ -7,7 +7,7 @@ describe('NodeError', () => {
 	const node = mock<INode>();
 
 	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);