refactor: Refactor axios invocation code, and add tests (no-changelog) (#12070)

packages/core/src/NodeExecuteFunctions.ts

@@ -13,13 +13,7 @@ import type {
 	OAuth2CredentialData,
 } from '@n8n/client-oauth2';
 import { ClientOAuth2 } from '@n8n/client-oauth2';
-import type {
+import type { AxiosError, AxiosHeaders, AxiosRequestConfig, AxiosResponse } from 'axios';
-	AxiosError,
-	AxiosHeaders,
-	AxiosPromise,
-	AxiosRequestConfig,
-	AxiosResponse,
-} from 'axios';
 import axios from 'axios';
 import crypto, { createHmac } from 'crypto';
 import FileType from 'file-type';
@@ -748,6 +742,26 @@ export async function binaryToString(body: Buffer | Readable, encoding?: string)
 	return iconv.decode(buffer, encoding ?? 'utf-8');
 }
 
+export async function invokeAxios(
+	axiosConfig: AxiosRequestConfig,
+	authOptions: IRequestOptions['auth'] = {},
+) {
+	try {
+		return await axios(axiosConfig);
+	} catch (error) {
+		if (authOptions.sendImmediately !== false || !(error instanceof axios.AxiosError)) throw error;
+		// for digest-auth
+		const { response } = error;
+		if (response?.status !== 401 || !response.headers['www-authenticate']?.includes('nonce')) {
+			throw error;
+		}
+		const { auth } = axiosConfig;
+		delete axiosConfig.auth;
+		axiosConfig = digestAuthAxiosConfig(axiosConfig, response, auth);
+		return await axios(axiosConfig);
+	}
+}
+
 export async function proxyRequestToAxios(
 	workflow: Workflow | undefined,
 	additionalData: IWorkflowExecuteAdditionalData | undefined,
@@ -768,29 +782,8 @@ export async function proxyRequestToAxios(
 
 	axiosConfig = Object.assign(axiosConfig, await parseRequestObject(configObject));
 
-	let requestFn: () => AxiosPromise;
-	if (configObject.auth?.sendImmediately === false) {
-		// for digest-auth
-		requestFn = async () => {
-			try {
-				return await axios(axiosConfig);
-			} catch (error) {
-				const { response } = error;
-				if (response?.status !== 401 || !response.headers['www-authenticate']?.includes('nonce')) {
-					throw error;
-				}
-				const { auth } = axiosConfig;
-				delete axiosConfig.auth;
-				axiosConfig = digestAuthAxiosConfig(axiosConfig, response, auth);
-				return await axios(axiosConfig);
-			}
-		};
-	} else {
-		requestFn = async () => await axios(axiosConfig);
-	}
-
 	try {
-		const response = await requestFn();
+		const response = await invokeAxios(axiosConfig, configObject.auth);
 		let body = response.data;
 		if (body instanceof IncomingMessage && axiosConfig.responseType === 'stream') {
 			parseIncomingMessage(body);
@@ -982,7 +975,7 @@ export async function httpRequest(
 ): Promise<IN8nHttpFullResponse | IN8nHttpResponse> {
 	removeEmptyBody(requestOptions);
 
-	let axiosRequest = convertN8nRequestToAxios(requestOptions);
+	const axiosRequest = convertN8nRequestToAxios(requestOptions);
 	if (
 		axiosRequest.data === undefined ||
 		(axiosRequest.method !== undefined && axiosRequest.method.toUpperCase() === 'GET')
@@ -990,23 +983,7 @@ export async function httpRequest(
 		delete axiosRequest.data;
 	}
 
-	let result: AxiosResponse<any>;
+	const result = await invokeAxios(axiosRequest, requestOptions.auth);
-	try {
-		result = await axios(axiosRequest);
-	} catch (error) {
-		if (requestOptions.auth?.sendImmediately === false) {
-			const { response } = error;
-			if (response?.status !== 401 || !response.headers['www-authenticate']?.includes('nonce')) {
-				throw error;
-			}
-
-			const { auth } = axiosRequest;
-			delete axiosRequest.auth;
-			axiosRequest = digestAuthAxiosConfig(axiosRequest, response, auth);
-			result = await axios(axiosRequest);
-		}
-		throw error;
-	}
 
 	if (requestOptions.returnFullResponse) {
 		return {

packages/core/test/NodeExecuteFunctions.test.ts

@@ -1,3 +1,4 @@
+import FormData from 'form-data';
 import { mkdtempSync, readFileSync } from 'fs';
 import { IncomingMessage } from 'http';
 import type { Agent } from 'https';
@@ -26,6 +27,7 @@ import {
 	binaryToString,
 	copyInputItems,
 	getBinaryDataBuffer,
+	invokeAxios,
 	isFilePathBlocked,
 	parseContentDisposition,
 	parseContentType,
@@ -543,6 +545,46 @@ describe('NodeExecuteFunctions', () => {
 	});
 
 	describe('parseRequestObject', () => {
+		test('should handle basic request options', async () => {
+			const axiosOptions = await parseRequestObject({
+				url: 'https://example.com',
+				method: 'POST',
+				headers: { 'content-type': 'application/json' },
+				body: { key: 'value' },
+			});
+
+			expect(axiosOptions).toEqual(
+				expect.objectContaining({
+					url: 'https://example.com',
+					method: 'POST',
+					headers: { accept: '*/*', 'content-type': 'application/json' },
+					data: { key: 'value' },
+					maxRedirects: 0,
+				}),
+			);
+		});
+
+		test('should set correct headers for FormData', async () => {
+			const formData = new FormData();
+			formData.append('key', 'value');
+
+			const axiosOptions = await parseRequestObject({
+				url: 'https://example.com',
+				formData,
+				headers: {
+					'content-type': 'multipart/form-data',
+				},
+			});
+
+			expect(axiosOptions.headers).toMatchObject({
+				accept: '*/*',
+				'content-length': 163,
+				'content-type': expect.stringMatching(/^multipart\/form-data; boundary=/),
+			});
+
+			expect(axiosOptions.data).toBeInstanceOf(FormData);
+		});
+
 		test('should not use Host header for SNI', async () => {
 			const axiosOptions = await parseRequestObject({
 				url: 'https://example.de/foo/bar',
@@ -628,6 +670,78 @@ describe('NodeExecuteFunctions', () => {
 		});
 	});
 
+	describe('invokeAxios', () => {
+		const baseUrl = 'http://example.de';
+
+		beforeEach(() => {
+			nock.cleanAll();
+			nock.disableNetConnect();
+			jest.clearAllMocks();
+		});
+
+		it('should throw error for non-401 status codes', async () => {
+			nock(baseUrl).get('/test').reply(500, {});
+
+			await expect(invokeAxios({ url: `${baseUrl}/test` })).rejects.toThrow(
+				'Request failed with status code 500',
+			);
+		});
+
+		it('should throw error on 401 without digest auth challenge', async () => {
+			nock(baseUrl).get('/test').reply(401, {});
+
+			await expect(
+				invokeAxios(
+					{
+						url: `${baseUrl}/test`,
+					},
+					{ sendImmediately: false },
+				),
+			).rejects.toThrow('Request failed with status code 401');
+		});
+
+		it('should make successful requests', async () => {
+			nock(baseUrl).get('/test').reply(200, { success: true });
+
+			const response = await invokeAxios({
+				url: `${baseUrl}/test`,
+			});
+
+			expect(response.status).toBe(200);
+			expect(response.data).toEqual({ success: true });
+		});
+
+		it('should handle digest auth when receiving 401 with nonce', async () => {
+			nock(baseUrl)
+				.get('/test')
+				.matchHeader('authorization', 'Basic dXNlcjpwYXNz')
+				.once()
+				.reply(401, {}, { 'www-authenticate': 'Digest realm="test", nonce="abc123", qop="auth"' });
+
+			nock(baseUrl)
+				.get('/test')
+				.matchHeader(
+					'authorization',
+					/^Digest username="user",realm="test",nonce="abc123",uri="\/test",qop="auth",algorithm="MD5",response="[0-9a-f]{32}"/,
+				)
+				.reply(200, { success: true });
+
+			const response = await invokeAxios(
+				{
+					url: `${baseUrl}/test`,
+					auth: {
+						username: 'user',
+						password: 'pass',
+					},
+				},
+				{ sendImmediately: false },
+			);
+
+			expect(response.status).toBe(200);
+			expect(response.data).toEqual({ success: true });
+		});
+	});
+
 	describe('copyInputItems', () => {
 		it('should pick only selected properties', () => {
 			const output = copyInputItems(