Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use ES2022 native error chaining to improve error reporting #4431

Merged
merged 1 commit into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,7 @@ export class ActiveWorkflowRunner {
// Run Error Workflow if defined
const activationError = new WorkflowActivationError(
`There was a problem with the trigger node "${node.name}", for that reason did the workflow had to be deactivated`,
error,
node,
{ cause: error, node },
);
this.executeErrorWorkflow(activationError, workflowData, mode);

Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/ExternalHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ class ExternalHooksClass implements IExternalHooksClass {
const hookFile = require(hookFilePath) as IExternalHooksFileData;
this.loadHooks(hookFile);
} catch (error) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions, @typescript-eslint/no-unsafe-member-access
throw new Error(`Problem loading external hook file "${hookFilePath}": ${error.message}`);
throw new Error(
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/restrict-template-expressions
`Problem loading external hook file "${hookFilePath}": ${error.message}`,
{ cause: error as Error },
);
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions packages/core/src/ActiveWorkflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ export class ActiveWorkflows {
throw new WorkflowActivationError(
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions, @typescript-eslint/no-unsafe-member-access
`There was a problem activating the workflow: "${error.message}"`,
error,
triggerNode,
{ cause: error as Error, node: triggerNode },
);
}
}
Expand All @@ -122,8 +121,7 @@ export class ActiveWorkflows {
throw new WorkflowActivationError(
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions, @typescript-eslint/no-unsafe-member-access
`There was a problem activating the workflow: "${error.message}"`,
error,
pollNode,
{ cause: error as Error, node: pollNode },
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nodes-base/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"outDir": "dist",
"lib": ["dom", "es2020"],
"lib": ["dom", "es2020", "es2022.error"],
"types": ["node", "jest"],
// TODO: remove all options below this line
"noImplicitReturns": false,
Expand Down
3 changes: 2 additions & 1 deletion packages/workflow/src/ExpressionError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export class ExpressionError extends ExecutionBaseError {
constructor(
message: string,
options?: {
cause?: Error;
causeDetailed?: string;
description?: string;
descriptionTemplate?: string;
Expand All @@ -22,7 +23,7 @@ export class ExpressionError extends ExecutionBaseError {
type?: string;
},
) {
super(new Error(message));
super(message, { cause: options?.cause });

if (options?.description !== undefined) {
this.description = options.description;
Expand Down
88 changes: 37 additions & 51 deletions packages/workflow/src/NodeErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,29 +56,28 @@ const ERROR_STATUS_PROPERTIES = [
*/
const ERROR_NESTING_PROPERTIES = ['error', 'err', 'response', 'body', 'data'];

interface ExecutionBaseErrorOptions {
cause?: Error | JsonObject;
}

export abstract class ExecutionBaseError extends Error {
description: string | null | undefined;

cause: Error | JsonObject;

timestamp: number;

context: IDataObject = {};

lineNumber: number | undefined;

constructor(error: Error | ExecutionBaseError | JsonObject) {
super();
constructor(message: string, { cause }: ExecutionBaseErrorOptions) {
const options = cause instanceof Error ? { cause } : {};
super(message, options);

this.name = this.constructor.name;
this.cause = error;
this.timestamp = Date.now();

if (error.message) {
this.message = error.message as string;
}

if (error instanceof ExecutionBaseError) {
this.context = error.context;
if (cause instanceof ExecutionBaseError) {
this.context = cause.context;
}
}
}
Expand All @@ -91,7 +90,8 @@ abstract class NodeError extends ExecutionBaseError {
node: INode;

constructor(node: INode, error: Error | JsonObject) {
super(error);
const message = error instanceof Error ? error.message : '';
super(message, { cause: error });
this.node = node;
}

Expand Down Expand Up @@ -120,23 +120,23 @@ abstract class NodeError extends ExecutionBaseError {
*
*/
protected findProperty(
error: JsonObject,
jsonError: JsonObject,
potentialKeys: string[],
traversalKeys: string[] = [],
): string | null {
// eslint-disable-next-line no-restricted-syntax
for (const key of potentialKeys) {
const value = error[key];
const value = jsonError[key];
if (value) {
if (typeof value === 'string') return value;
if (typeof value === 'number') return value.toString();
if (Array.isArray(value)) {
const resolvedErrors: string[] = value
.map((error) => {
if (typeof error === 'string') return error;
if (typeof error === 'number') return error.toString();
if (this.isTraversableObject(error)) {
return this.findProperty(error, potentialKeys);
.map((jsonError) => {
if (typeof jsonError === 'string') return jsonError;
if (typeof jsonError === 'number') return jsonError.toString();
if (this.isTraversableObject(jsonError)) {
return this.findProperty(jsonError, potentialKeys);
}
return null;
})
Expand All @@ -158,7 +158,7 @@ abstract class NodeError extends ExecutionBaseError {

// eslint-disable-next-line no-restricted-syntax
for (const key of traversalKeys) {
const value = error[key];
const value = jsonError[key];
if (this.isTraversableObject(value)) {
const property = this.findProperty(value, potentialKeys, traversalKeys);
if (property) {
Expand Down Expand Up @@ -209,33 +209,27 @@ abstract class NodeError extends ExecutionBaseError {
}
}

interface NodeOperationErrorOptions {
description?: string;
runIndex?: number;
itemIndex?: number;
}

/**
* Class for instantiating an operational error, e.g. an invalid credentials error.
*/
export class NodeOperationError extends NodeError {
lineNumber: number | undefined;

constructor(
node: INode,
error: Error | string,
options?: { description?: string; runIndex?: number; itemIndex?: number },
) {
constructor(node: INode, error: Error | string, options: NodeOperationErrorOptions = {}) {
if (typeof error === 'string') {
error = new Error(error);
}
super(node, error);

if (options?.description) {
this.description = options.description;
}

if (options?.runIndex !== undefined) {
this.context.runIndex = options.runIndex;
}

if (options?.itemIndex !== undefined) {
this.context.itemIndex = options.itemIndex;
}
this.description = options.description;
this.context.runIndex = options.runIndex;
this.context.itemIndex = options.itemIndex;
}
}

Expand All @@ -258,6 +252,12 @@ const STATUS_CODE_MESSAGES: IStatusCodeMessages = {

const UNKNOWN_ERROR_MESSAGE = 'UNKNOWN ERROR - check the detailed error for more information';

interface NodeApiErrorOptions extends NodeOperationErrorOptions {
message?: string;
httpCode?: string;
parseXml?: boolean;
}

/**
* 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.
Expand All @@ -268,21 +268,7 @@ export class NodeApiError extends NodeError {
constructor(
node: INode,
error: JsonObject,
{
message,
description,
httpCode,
parseXml,
runIndex,
itemIndex,
}: {
message?: string;
description?: string;
httpCode?: string;
parseXml?: boolean;
runIndex?: number;
itemIndex?: number;
} = {},
{ message, description, httpCode, parseXml, runIndex, itemIndex }: NodeApiErrorOptions = {},
) {
super(node, error);
if (error.error) {
Expand Down
12 changes: 6 additions & 6 deletions packages/workflow/src/RoutingNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,12 @@ export class RoutingNode {
};
});
});
} catch (e) {
throw new NodeOperationError(
this.node,
`The rootProperty "${action.properties.property}" could not be found on item.`,
{ runIndex, itemIndex },
);
} catch (error) {
throw new NodeOperationError(this.node, error, {
runIndex,
itemIndex,
description: `The rootProperty "${action.properties.property}" could not be found on item.`,
});
}
}
if (action.type === 'limit') {
Expand Down
20 changes: 14 additions & 6 deletions packages/workflow/src/WorkflowActivationError.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
import { INode } from './Interfaces';
import { ExecutionBaseError } from './NodeErrors';

interface WorkflowActivationErrorOptions {
cause?: Error;
node?: INode;
}

/**
* Class for instantiating an workflow activation error
*/
export class WorkflowActivationError extends ExecutionBaseError {
node: INode | undefined;

constructor(message: string, error: Error, node?: INode) {
super(error);
constructor(message: string, { cause, node }: WorkflowActivationErrorOptions) {
let error = cause as Error;
if (cause instanceof ExecutionBaseError) {
error = new Error(cause.message);
error.constructor = cause.constructor;
error.name = cause.name;
error.stack = cause.stack;
}
super(message, { cause: error });
this.node = node;
this.cause = {
message: error.message,
stack: error.stack as string,
};
this.message = message;
}
}
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"module": "commonjs",
"moduleResolution": "node",
"target": "es2019",
"lib": ["es2019", "es2020"],
"lib": ["es2019", "es2020", "es2022.error"],
"removeComments": true,
"useUnknownInCatchVariables": true,
"forceConsistentCasingInFileNames": true,
Expand Down