fix(HTTP Request Node): Show detailed error message in the UI again (#5959)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2023-04-12 14:58:05 +02:00 committed by GitHub
parent 60d28fc761
commit e79679c023
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 141 additions and 38 deletions

View file

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

View file

@ -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']),
});
}

View file

@ -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,
]);
});
});
});

View file

@ -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 () => {

View file

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

View file

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

View file

@ -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({