fix(HTTP Request Node): Show detailed error message in the UI again (n…

…8n-io#5959)

packages/core/jest.config.js

@@ -1,2 +1,5 @@
 /** @type {import('jest').Config} */
-module.exports = require('../../jest.config');
+module.exports = {
+	...require('../../jest.config'),
+	globalSetup: '<rootDir>/test/setup.ts',
+};

packages/core/src/NodeExecuteFunctions.ts

@@ -578,7 +578,13 @@ function digestAuthAxiosConfig(
 	return axiosConfig;
 }
 
-async function proxyRequestToAxios(
+type ConfigObject = {
+	auth?: { sendImmediately: boolean };
+	resolveWithFullResponse?: boolean;
+	simple?: boolean;
+};
+
+export async function proxyRequestToAxios(
 	workflow: Workflow,
 	additionalData: IWorkflowExecuteAdditionalData,
 	node: INode,
@@ -598,12 +604,6 @@ async function proxyRequestToAxios(
 		maxBodyLength: Infinity,
 		maxContentLength: Infinity,
 	};
-
-	type ConfigObject = {
-		auth?: { sendImmediately: boolean };
-		resolveWithFullResponse?: boolean;
-		simple?: boolean;
-	};
 	let configObject: ConfigObject;
 	if (uriOrObject !== undefined && typeof uriOrObject === 'string') {
 		axiosConfig.url = uriOrObject;
@@ -707,12 +707,17 @@ async function proxyRequestToAxios(
 				}
 
 				const message = `${response.status as number} - ${JSON.stringify(responseData)}`;
-				throw Object.assign(new Error(message, { cause: error }), {
+				throw Object.assign(error, {
+					message,
 					statusCode: response.status,
 					options: pick(config ?? {}, ['url', 'method', 'data', 'headers']),
+					error: responseData,
+					config: undefined,
+					request: undefined,
+					response: pick(response, ['headers', 'status', 'statusText']),
 				});
 			} else {
-				throw Object.assign(new Error(error.message, { cause: error }), {
+				throw Object.assign(error, {
 					options: pick(config ?? {}, ['url', 'method', 'data', 'headers']),
 				});
 			}

packages/core/test/NodeExecuteFunctions.test.ts

@@ -1,15 +1,28 @@
+import nock from 'nock';
 import { join } from 'path';
 import { tmpdir } from 'os';
 import { readFileSync, mkdtempSync } from 'fs';
-
-import { IBinaryData, ITaskDataConnections } from 'n8n-workflow';
+import { mock } from 'jest-mock-extended';
+import type {
+	IBinaryData,
+	INode,
+	ITaskDataConnections,
+	IWorkflowExecuteAdditionalData,
+	Workflow,
+	WorkflowHooks,
+} from 'n8n-workflow';
 import { BinaryDataManager } from '@/BinaryDataManager';
-import * as NodeExecuteFunctions from '@/NodeExecuteFunctions';
+import {
+	setBinaryDataBuffer,
+	getBinaryDataBuffer,
+	proxyRequestToAxios,
+} from '@/NodeExecuteFunctions';
+import { initLogger } from './utils';
 
 const temporaryDir = mkdtempSync(join(tmpdir(), 'n8n'));
 
 describe('NodeExecuteFunctions', () => {
-	describe(`test binary data helper methods`, () => {
+	describe('test binary data helper methods', () => {
 		// Reset BinaryDataManager for each run. This is a dirty operation, as individual managers are not cleaned.
 		beforeEach(() => {
 			BinaryDataManager.instance = undefined;
@@ -27,7 +40,7 @@ describe('NodeExecuteFunctions', () => {
 
 			// Set our binary data buffer
 			let inputData: Buffer = Buffer.from('This is some binary data', 'utf8');
-			let setBinaryDataBufferResponse: IBinaryData = await NodeExecuteFunctions.setBinaryDataBuffer(
+			let setBinaryDataBufferResponse: IBinaryData = await setBinaryDataBuffer(
 				{
 					mimeType: 'txt',
 					data: 'This should be overwritten by the actual payload in the response',
@@ -56,7 +69,7 @@ describe('NodeExecuteFunctions', () => {
 			]);
 
 			// Now, lets fetch our data! The item will be item index 0.
-			let getBinaryDataBufferResponse: Buffer = await NodeExecuteFunctions.getBinaryDataBuffer(
+			let getBinaryDataBufferResponse: Buffer = await getBinaryDataBuffer(
 				taskDataConnectionsInput,
 				0,
 				'data',
@@ -78,7 +91,7 @@ describe('NodeExecuteFunctions', () => {
 
 			// Set our binary data buffer
 			let inputData: Buffer = Buffer.from('This is some binary data', 'utf8');
-			let setBinaryDataBufferResponse: IBinaryData = await NodeExecuteFunctions.setBinaryDataBuffer(
+			let setBinaryDataBufferResponse: IBinaryData = await setBinaryDataBuffer(
 				{
 					mimeType: 'txt',
 					data: 'This should be overwritten with the name of the configured data manager',
@@ -114,7 +127,7 @@ describe('NodeExecuteFunctions', () => {
 			]);
 
 			// Now, lets fetch our data! The item will be item index 0.
-			let getBinaryDataBufferResponse: Buffer = await NodeExecuteFunctions.getBinaryDataBuffer(
+			let getBinaryDataBufferResponse: Buffer = await getBinaryDataBuffer(
 				taskDataConnectionsInput,
 				0,
 				'data',
@@ -124,4 +137,80 @@ describe('NodeExecuteFunctions', () => {
 			expect(getBinaryDataBufferResponse).toEqual(inputData);
 		});
 	});
+
+	describe('proxyRequestToAxios', () => {
+		const baseUrl = 'http://example.de';
+		const workflow = mock<Workflow>();
+		const hooks = mock<WorkflowHooks>();
+		const additionalData = mock<IWorkflowExecuteAdditionalData>({ hooks });
+		const node = mock<INode>();
+
+		beforeEach(() => {
+			initLogger();
+			hooks.executeHookFunctions.mockClear();
+		});
+
+		test('should not throw if the response status is 200', async () => {
+			nock(baseUrl).get('/test').reply(200);
+			await proxyRequestToAxios(workflow, additionalData, node, `${baseUrl}/test`);
+			expect(hooks.executeHookFunctions).toHaveBeenCalledWith('nodeFetchedData', [
+				workflow.id,
+				node,
+			]);
+		});
+
+		test('should throw if the response status is 403', async () => {
+			const headers = { 'content-type': 'text/plain' };
+			nock(baseUrl).get('/test').reply(403, 'Forbidden', headers);
+			try {
+				await proxyRequestToAxios(workflow, additionalData, node, `${baseUrl}/test`);
+			} catch (error) {
+				expect(error.statusCode).toEqual(403);
+				expect(error.request).toBeUndefined();
+				expect(error.response).toMatchObject({ headers, status: 403 });
+				expect(error.options).toMatchObject({
+					headers: { Accept: '*/*' },
+					method: 'get',
+					url: 'http://example.de/test',
+				});
+				expect(error.config).toBeUndefined();
+				expect(error.message).toEqual('403 - "Forbidden"');
+			}
+			expect(hooks.executeHookFunctions).not.toHaveBeenCalled();
+		});
+
+		test('should not throw if the response status is 404, but `simple` option is set to `false`', async () => {
+			nock(baseUrl).get('/test').reply(404, 'Not Found');
+			const response = await proxyRequestToAxios(workflow, additionalData, node, {
+				url: `${baseUrl}/test`,
+				simple: false,
+			});
+
+			expect(response).toEqual('Not Found');
+			expect(hooks.executeHookFunctions).toHaveBeenCalledWith('nodeFetchedData', [
+				workflow.id,
+				node,
+			]);
+		});
+
+		test('should return full response when `resolveWithFullResponse` is set to true', async () => {
+			nock(baseUrl).get('/test').reply(404, 'Not Found');
+			const response = await proxyRequestToAxios(workflow, additionalData, node, {
+				url: `${baseUrl}/test`,
+				resolveWithFullResponse: true,
+				simple: false,
+			});
+
+			expect(response).toMatchObject({
+				body: 'Not Found',
+				headers: {},
+				statusCode: 404,
+				statusMessage: null,
+			});
+			expect(hooks.executeHookFunctions).toHaveBeenCalledWith('nodeFetchedData', [
+				workflow.id,
+				node,
+			]);
+		});
+	});
 });

packages/core/test/WorkflowExecute.test.ts

@@ -1,17 +1,14 @@
-import {
-	createDeferredPromise,
-	IConnections,
-	ILogger,
-	INode,
-	IRun,
-	LoggerProxy,
-	Workflow,
-} from 'n8n-workflow';
+import { createDeferredPromise, IConnections, INode, IRun, Workflow } from 'n8n-workflow';
 import { WorkflowExecute } from '@/WorkflowExecute';
 
 import * as Helpers from './Helpers';
+import { initLogger } from './utils';
 
 describe('WorkflowExecute', () => {
+	beforeAll(() => {
+		initLogger();
+	});
+
 	describe('run', () => {
 		const tests: Array<{
 			description: string;
@@ -1352,18 +1349,8 @@ describe('WorkflowExecute', () => {
 			},
 		];
 
-		const fakeLogger = {
-			log: () => {},
-			debug: () => {},
-			verbose: () => {},
-			info: () => {},
-			warn: () => {},
-			error: () => {},
-		} as ILogger;
-
 		const executionMode = 'manual';
 		const nodeTypes = Helpers.NodeTypes();
-		LoggerProxy.init(fakeLogger);
 
 		for (const testData of tests) {
 			test(testData.description, async () => {

packages/core/test/setup.ts

@@ -0,0 +1,5 @@
+import nock from 'nock';
+
+export default async () => {
+	nock.disableNetConnect();
+};

packages/core/test/utils.ts

@@ -0,0 +1,14 @@
+import { ILogger, LoggerProxy } from 'n8n-workflow';
+
+const fakeLogger = {
+	log: () => {},
+	debug: () => {},
+	verbose: () => {},
+	info: () => {},
+	warn: () => {},
+	error: () => {},
+} as ILogger;
+
+export const initLogger = () => {
+	LoggerProxy.init(fakeLogger);
+};

packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts

@@ -1369,7 +1369,7 @@ export class HttpRequestV3 implements INodeType {
 					if (autoDetectResponseFormat && response.reason.error instanceof Buffer) {
 						response.reason.error = Buffer.from(response.reason.error as Buffer).toString();
 					}
-					throw new NodeApiError(this.getNode(), response.reason as JsonObject);
+					throw new NodeApiError(this.getNode(), response as JsonObject);
 				} else {
 					// Return the actual reason as error
 					returnItems.push({