refactor(core): Separate API response from error in execution error causes (no-changelog) (#7880)

Store the third-party API response error separately from the error
stored as `cause`

Follow-up to:
https://github.com/n8n-io/n8n/pull/7820#discussion_r1406009154
This commit is contained in:
Iván Ovejero 2023-11-30 14:44:10 +01:00 committed by GitHub
parent b024cc52e7
commit e0b7f89035
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 56 additions and 33 deletions

View file

@ -196,7 +196,12 @@ export class MailchimpTrigger implements INodeType {
try { try {
await mailchimpApiRequest.call(this, endpoint, 'GET'); await mailchimpApiRequest.call(this, endpoint, 'GET');
} catch (error) { } catch (error) {
if (error instanceof NodeApiError && error.cause && 'isAxiosError' in error.cause) { if (
error instanceof NodeApiError &&
error.cause &&
'isAxiosError' in error.cause &&
'statusCode' in error.cause
) {
if (error.cause.statusCode === 404) { if (error.cause.statusCode === 404) {
return false; return false;
} }

View file

@ -3,6 +3,7 @@ import type {
INodeExecutionData, INodeExecutionData,
INodeType, INodeType,
INodeTypeDescription, INodeTypeDescription,
JsonObject,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { jsonParse, NodeOperationError } from 'n8n-workflow'; import { jsonParse, NodeOperationError } from 'n8n-workflow';
@ -81,14 +82,14 @@ export class StopAndError implements INodeType {
const errorType = this.getNodeParameter('errorType', 0) as 'errorMessage' | 'errorObject'; const errorType = this.getNodeParameter('errorType', 0) as 'errorMessage' | 'errorObject';
const { id: workflowId, name: workflowName } = this.getWorkflow(); const { id: workflowId, name: workflowName } = this.getWorkflow();
let toThrow: string | { name: string; message: string; [otherKey: string]: unknown }; let toThrow: string | JsonObject;
if (errorType === 'errorMessage') { if (errorType === 'errorMessage') {
toThrow = this.getNodeParameter('errorMessage', 0) as string; toThrow = this.getNodeParameter('errorMessage', 0) as string;
} else { } else {
const json = this.getNodeParameter('errorObject', 0) as string; const json = this.getNodeParameter('errorObject', 0) as string;
const errorObject = jsonParse<any>(json); const errorObject = jsonParse<JsonObject>(json);
toThrow = { toThrow = {
name: 'User-thrown error', name: 'User-thrown error',

View file

@ -88,7 +88,7 @@ describe('Execute Stop and Error Node', () => {
const stopAndError1RunData = result.data.resultData.runData['Stop and Error1']; const stopAndError1RunData = result.data.resultData.runData['Stop and Error1'];
const stopAndError1Object = ( const stopAndError1Object = (
(stopAndError1RunData as unknown as IDataObject[])[0].error as IDataObject (stopAndError1RunData as unknown as IDataObject[])[0].error as IDataObject
).cause; ).errorResponse;
expect(stopAndError1Object).toEqual({ expect(stopAndError1Object).toEqual({
code: 404, code: 404,

View file

@ -2,16 +2,16 @@ import type { Functionality, IDataObject, JsonObject, Severity } from '../../Int
import { ApplicationError } from '../application.error'; import { ApplicationError } from '../application.error';
interface ExecutionBaseErrorOptions { interface ExecutionBaseErrorOptions {
cause?: Error | JsonObject; cause?: Error;
errorResponse?: JsonObject;
} }
export abstract class ExecutionBaseError extends ApplicationError { export abstract class ExecutionBaseError extends ApplicationError {
description: string | null | undefined; description: string | null | undefined;
/** cause?: Error;
* @tech_debt Ensure `cause` can only be `Error` or `undefined`
*/ errorResponse?: JsonObject;
cause: Error | JsonObject | undefined;
timestamp: number; timestamp: number;
@ -23,7 +23,7 @@ export abstract class ExecutionBaseError extends ApplicationError {
functionality: Functionality = 'regular'; functionality: Functionality = 'regular';
constructor(message: string, { cause }: ExecutionBaseErrorOptions) { constructor(message: string, { cause, errorResponse }: ExecutionBaseErrorOptions = {}) {
const options = cause instanceof Error ? { cause } : {}; const options = cause instanceof Error ? { cause } : {};
super(message, options); super(message, options);
@ -35,6 +35,8 @@ export abstract class ExecutionBaseError extends ApplicationError {
} else if (cause && !(cause instanceof Error)) { } else if (cause && !(cause instanceof Error)) {
this.cause = cause; this.cause = cause;
} }
if (errorResponse) this.errorResponse = errorResponse;
} }
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any

View file

@ -38,8 +38,12 @@ export abstract class NodeError extends ExecutionBaseError {
node: INode; node: INode;
constructor(node: INode, error: Error | JsonObject) { constructor(node: INode, error: Error | JsonObject) {
const message = error instanceof Error ? error.message : ''; if (error instanceof Error) {
super(message, { cause: error }); super(error.message, { cause: error });
} else {
super('', { errorResponse: error });
}
this.node = node; this.node = node;
} }

View file

@ -112,7 +112,7 @@ export class NodeApiError extends NodeError {
constructor( constructor(
node: INode, node: INode,
error: JsonObject, errorResponse: JsonObject,
{ {
message, message,
description, description,
@ -125,38 +125,44 @@ export class NodeApiError extends NodeError {
messageMapping, messageMapping,
}: NodeApiErrorOptions = {}, }: NodeApiErrorOptions = {},
) { ) {
super(node, error); super(node, errorResponse);
// only for request library error // only for request library error
if (error.error) { if (errorResponse.error) {
removeCircularRefs(error.error as JsonObject); removeCircularRefs(errorResponse.error as JsonObject);
} }
// if not description provided, try to find it in the error object // if not description provided, try to find it in the error object
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (!description && (error.description || (error?.reason as IDataObject)?.description)) { if (
!description &&
(errorResponse.description || (errorResponse?.reason as IDataObject)?.description)
) {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
this.description = (error.description || this.description = (errorResponse.description ||
(error?.reason as IDataObject)?.description) as string; (errorResponse?.reason as IDataObject)?.description) as string;
} }
// if not message provided, try to find it in the error object or set description as message // if not message provided, try to find it in the error object or set description as message
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (!message && (error.message || (error?.reason as IDataObject)?.message || description)) { if (
!message &&
(errorResponse.message || (errorResponse?.reason as IDataObject)?.message || description)
) {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
this.message = (error.message || this.message = (errorResponse.message ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
(error?.reason as IDataObject)?.message || (errorResponse?.reason as IDataObject)?.message ||
description) as string; description) as string;
} }
// if it's an error generated by axios // if it's an error generated by axios
// look for descriptions in the response object // look for descriptions in the response object
if (error.reason) { if (errorResponse.reason) {
const reason: IDataObject = error.reason as unknown as IDataObject; const reason: IDataObject = errorResponse.reason as unknown as IDataObject;
if (reason.isAxiosError && reason.response) { if (reason.isAxiosError && reason.response) {
error = reason.response as JsonObject; errorResponse = reason.response as JsonObject;
} }
} }
@ -165,7 +171,7 @@ export class NodeApiError extends NodeError {
this.httpCode = httpCode; this.httpCode = httpCode;
} else { } else {
this.httpCode = this.httpCode =
this.findProperty(error, ERROR_STATUS_PROPERTIES, ERROR_NESTING_PROPERTIES) ?? null; this.findProperty(errorResponse, ERROR_STATUS_PROPERTIES, ERROR_NESTING_PROPERTIES) ?? null;
} }
if (severity) { if (severity) {
@ -181,10 +187,10 @@ export class NodeApiError extends NodeError {
if (!this.description) { if (!this.description) {
if (parseXml) { if (parseXml) {
this.setDescriptionFromXml(error.error as string); this.setDescriptionFromXml(errorResponse.error as string);
} else { } else {
this.description = this.findProperty( this.description = this.findProperty(
error, errorResponse,
ERROR_MESSAGE_PROPERTIES, ERROR_MESSAGE_PROPERTIES,
ERROR_NESTING_PROPERTIES, ERROR_NESTING_PROPERTIES,
); );
@ -209,8 +215,8 @@ export class NodeApiError extends NodeError {
this.description, this.description,
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
this.httpCode || this.httpCode ||
(error?.code as string) || (errorResponse?.code as string) ||
((error?.reason as JsonObject)?.code as string) || ((errorResponse?.reason as JsonObject)?.code as string) ||
undefined, undefined,
messageMapping, messageMapping,
); );

View file

@ -1,4 +1,4 @@
import type { INode } from '..'; import type { INode, JsonObject } from '..';
import type { NodeOperationErrorOptions } from './node-api.error'; import type { NodeOperationErrorOptions } from './node-api.error';
import { NodeError } from './abstract/node.error'; import { NodeError } from './abstract/node.error';
@ -8,7 +8,11 @@ import { NodeError } from './abstract/node.error';
export class NodeOperationError extends NodeError { export class NodeOperationError extends NodeError {
lineNumber: number | undefined; lineNumber: number | undefined;
constructor(node: INode, error: Error | string, options: NodeOperationErrorOptions = {}) { constructor(
node: INode,
error: Error | string | JsonObject,
options: NodeOperationErrorOptions = {},
) {
if (typeof error === 'string') { if (typeof error === 'string') {
error = new Error(error); error = new Error(error);
} }

View file

@ -3,7 +3,7 @@ import { WorkflowOperationError } from './workflow-operation.error';
export class SubworkflowOperationError extends WorkflowOperationError { export class SubworkflowOperationError extends WorkflowOperationError {
description = ''; description = '';
cause: { message: string; stack: string }; cause: Error;
constructor(message: string, description: string) { constructor(message: string, description: string) {
super(message); super(message);
@ -11,6 +11,7 @@ export class SubworkflowOperationError extends WorkflowOperationError {
this.description = description; this.description = description;
this.cause = { this.cause = {
name: this.name,
message, message,
stack: this.stack as string, stack: this.stack as string,
}; };