Skip to content

Commit

Permalink
refactor(core): Switch plain errors in workflow to `ApplicationErro…
Browse files Browse the repository at this point in the history
…r` (no-changelog) (#7877)

Ensure all errors in `workflow` are `ApplicationError` or children of it
and contain no variables in the message, to continue normalizing all the
errors we report to Sentry

Follow-up to: #7873
  • Loading branch information
ivov authored Nov 30, 2023
1 parent ce2d388 commit 67702c2
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 61 deletions.
2 changes: 1 addition & 1 deletion packages/workflow/src/ErrorReporterProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function init(errorReporter: ErrorReporter) {

const wrap = (e: unknown) => {
if (e instanceof Error) return e;
if (typeof e === 'string') return new Error(e);
if (typeof e === 'string') return new ApplicationError(e);
return;
};

Expand Down
13 changes: 7 additions & 6 deletions packages/workflow/src/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { extendedFunctions } from './Extensions/ExtendedFunctions';
import { extendSyntax } from './Extensions/ExpressionExtension';
import { evaluateExpression, setErrorHandler } from './ExpressionEvaluatorProxy';
import { getGlobalState } from './GlobalState';
import { ApplicationError } from './errors/application.error';

const IS_FRONTEND_IN_DEV_MODE =
typeof process === 'object' &&
Expand Down Expand Up @@ -75,7 +76,7 @@ export class Expression {
const typeName = Array.isArray(value) ? 'Array' : 'Object';

if (value instanceof DateTime && value.invalidReason !== null) {
throw new Error('invalid DateTime');
throw new ApplicationError('invalid DateTime');
}

let result = '';
Expand Down Expand Up @@ -310,12 +311,12 @@ export class Expression {
const extendedExpression = extendSyntax(parameterValue);
const returnValue = this.renderExpression(extendedExpression, data);
if (typeof returnValue === 'function') {
if (returnValue.name === '$') throw new Error('invalid syntax');
if (returnValue.name === '$') throw new ApplicationError('invalid syntax');

if (returnValue.name === 'DateTime')
throw new Error('this is a DateTime, please access its methods');
throw new ApplicationError('this is a DateTime, please access its methods');

throw new Error('this is a function, please add ()');
throw new ApplicationError('this is a function, please add ()');
} else if (typeof returnValue === 'string') {
return returnValue;
} else if (returnValue !== null && typeof returnValue === 'object') {
Expand All @@ -339,14 +340,14 @@ export class Expression {
} catch (error) {
if (isExpressionError(error)) throw error;

if (isSyntaxError(error)) throw new Error('invalid syntax');
if (isSyntaxError(error)) throw new ApplicationError('invalid syntax');

if (isTypeError(error) && IS_FRONTEND && error.message.endsWith('is not a function')) {
const match = error.message.match(/(?<msg>[^.]+is not a function)/);

if (!match?.groups?.msg) return null;

throw new Error(match.groups.msg);
throw new ApplicationError(match.groups.msg);
}
} finally {
Object.defineProperty(Function.prototype, 'constructor', { value: fnConstructors.sync });
Expand Down
46 changes: 29 additions & 17 deletions packages/workflow/src/NodeHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { deepCopy } from './utils';

import { DateTime } from 'luxon';
import type { Workflow } from './Workflow';
import { ApplicationError } from './errors/application.error';

export const cronNodeOptions: INodePropertyCollection[] = [
{
Expand Down Expand Up @@ -416,19 +417,24 @@ export function getContext(
): IContextObject {
if (runExecutionData.executionData === undefined) {
// TODO: Should not happen leave it for test now
throw new Error('The "executionData" is not initialized!');
throw new ApplicationError('`executionData` is not initialized');
}

let key: string;
if (type === 'flow') {
key = 'flow';
} else if (type === 'node') {
if (node === undefined) {
throw new Error('The request data of context type "node" the node parameter has to be set!');
// @TODO: What does this mean?
throw new ApplicationError(
'The request data of context type "node" the node parameter has to be set!',
);
}
key = `node:${node.name}`;
} else {
throw new Error(`The context type "${type}" is not know. Only "flow" and node" are supported!`);
throw new ApplicationError('Unknown context type. Only `flow` and `node` are supported.', {
extra: { contextType: type },
});
}

if (runExecutionData.executionData.contextData[key] === undefined) {
Expand Down Expand Up @@ -530,7 +536,7 @@ export function getParameterResolveOrder(
}

if (iterations > lastIndexReduction + nodePropertiesArray.length) {
throw new Error(
throw new ApplicationError(
'Could not resolve parameter dependencies. Max iterations reached! Hint: If `displayOptions` are specified in any child parameter of a parent `collection` or `fixedCollection`, remove the `displayOptions` from the child parameter.',
);
}
Expand Down Expand Up @@ -773,9 +779,9 @@ export function getNodeParameters(
) as INodePropertyCollection;

if (nodePropertyOptions === undefined) {
throw new Error(
`Could not find property option "${itemName}" for "${nodeProperties.name}"`,
);
throw new ApplicationError('Could not find property option', {
extra: { propertyOption: itemName, property: nodeProperties.name },
});
}

tempNodePropertiesArray = nodePropertyOptions.values!;
Expand Down Expand Up @@ -1054,7 +1060,9 @@ export function getNodeInputs(
{},
) || []) as ConnectionTypes[];
} catch (e) {
throw new Error(`Could not calculate inputs dynamically for node "${node.name}"`);
throw new ApplicationError('Could not calculate inputs dynamically for node', {
extra: { nodeName: node.name },
});
}
}

Expand All @@ -1077,7 +1085,9 @@ export function getNodeOutputs(
{},
) || []) as ConnectionTypes[];
} catch (e) {
throw new Error(`Could not calculate outputs dynamically for node "${node.name}"`);
throw new ApplicationError('Could not calculate outputs dynamically for node', {
extra: { nodeName: node.name },
});
}
}

Expand Down Expand Up @@ -1258,7 +1268,7 @@ export const tryToParseNumber = (value: unknown): number => {
const isValidNumber = !isNaN(Number(value));

if (!isValidNumber) {
throw new Error(`Could not parse '${String(value)}' to number.`);
throw new ApplicationError('Failed to parse value to number', { extra: { value } });
}
return Number(value);
};
Expand All @@ -1282,7 +1292,9 @@ export const tryToParseBoolean = (value: unknown): value is boolean => {
}
}

throw new Error(`Could not parse '${String(value)}' to boolean.`);
throw new ApplicationError('Failed to parse value as boolean', {
extra: { value },
});
};

export const tryToParseDateTime = (value: unknown): DateTime => {
Expand All @@ -1306,15 +1318,15 @@ export const tryToParseDateTime = (value: unknown): DateTime => {
return sqlDate;
}

throw new Error(`The value "${dateString}" is not a valid date.`);
throw new ApplicationError('Value is not a valid date', { extra: { dateString } });
};

export const tryToParseTime = (value: unknown): string => {
const isTimeInput = /^\d{2}:\d{2}(:\d{2})?((\-|\+)\d{4})?((\-|\+)\d{1,2}(:\d{2})?)?$/s.test(
String(value),
);
if (!isTimeInput) {
throw new Error(`The value "${String(value)}" is not a valid time.`);
throw new ApplicationError('Value is not a valid time', { extra: { value } });
}
return String(value);
};
Expand All @@ -1333,11 +1345,11 @@ export const tryToParseArray = (value: unknown): unknown[] => {
}

if (!Array.isArray(parsed)) {
throw new Error(`The value "${String(value)}" is not a valid array.`);
throw new ApplicationError('Value is not a valid array', { extra: { value } });
}
return parsed;
} catch (e) {
throw new Error(`The value "${String(value)}" is not a valid array.`);
throw new ApplicationError('Value is not a valid array', { extra: { value } });
}
};

Expand All @@ -1348,11 +1360,11 @@ export const tryToParseObject = (value: unknown): object => {
try {
const o = JSON.parse(String(value));
if (typeof o !== 'object' || Array.isArray(o)) {
throw new Error(`The value "${String(value)}" is not a valid object.`);
throw new ApplicationError('Value is not a valid object', { extra: { value } });
}
return o;
} catch (e) {
throw new Error(`The value "${String(value)}" is not a valid object.`);
throw new ApplicationError('Value is not a valid object', { extra: { value } });
}
};

Expand Down
3 changes: 2 additions & 1 deletion packages/workflow/src/TelemetryHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
INodeTypes,
INodeType,
} from './Interfaces';
import { ApplicationError } from './errors/application.error';

const STICKY_NODE_TYPE = 'n8n-nodes-base.stickyNote';

Expand Down Expand Up @@ -95,7 +96,7 @@ export function getDomainPath(raw: string, urlParts = URL_PARTS_REGEX): string {
try {
const url = new URL(raw);

if (!url.hostname) throw new Error('Malformed URL');
if (!url.hostname) throw new ApplicationError('Malformed URL');

return sanitizeRoute(url.pathname);
} catch {
Expand Down
50 changes: 34 additions & 16 deletions packages/workflow/src/Workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import * as ObservableObject from './ObservableObject';
import { RoutingNode } from './RoutingNode';
import { Expression } from './Expression';
import { NODES_WITH_RENAMABLE_CONTENT } from './Constants';
import { ApplicationError } from './errors/application.error';

function dedupe<T>(arr: T[]): T[] {
return [...new Set(arr)];
Expand Down Expand Up @@ -113,7 +114,10 @@ export class Workflow {
// expression resolution also then when the unknown node
// does not get used.
continue;
// throw new Error(`The node type "${node.type}" of node "${node.name}" is not known.`);
// throw new ApplicationError(`Node with unknown node type`, {
// tags: { nodeType: node.type },
// extra: { node },
// });
}

// Add default values
Expand Down Expand Up @@ -312,15 +316,15 @@ export class Workflow {
key = 'global';
} else if (type === 'node') {
if (node === undefined) {
throw new Error(
throw new ApplicationError(
'The request data of context type "node" the node parameter has to be set!',
);
}
key = `node:${node.name}`;
} else {
throw new Error(
`The context type "${type}" is not know. Only "global" and node" are supported!`,
);
throw new ApplicationError('Unknown context type. Only `global` and `node` are supported.', {
extra: { contextType: type },
});
}

if (this.staticData[key] === undefined) {
Expand Down Expand Up @@ -1092,13 +1096,17 @@ export class Workflow {
const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);

if (nodeType === undefined) {
throw new Error(`The node type "${node.type}" of node "${node.name}" is not known.`);
throw new ApplicationError('Node with unknown node type', {
extra: { nodeName: node.name },
tags: { nodeType: node.type },
});
}

if (!nodeType.trigger) {
throw new Error(
`The node type "${node.type}" of node "${node.name}" does not have a trigger function defined.`,
);
throw new ApplicationError('Node type does not have a trigger function defined', {
extra: { nodeName: node.name },
tags: { nodeType: node.type },
});
}

if (mode === 'manual') {
Expand Down Expand Up @@ -1169,13 +1177,17 @@ export class Workflow {
const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);

if (nodeType === undefined) {
throw new Error(`The node type "${node.type}" of node "${node.name}" is not known.`);
throw new ApplicationError('Node with unknown node type', {
extra: { nodeName: node.name },
tags: { nodeType: node.type },
});
}

if (!nodeType.poll) {
throw new Error(
`The node type "${node.type}" of node "${node.name}" does not have a poll function defined.`,
);
throw new ApplicationError('Node type does not have a poll function defined', {
extra: { nodeName: node.name },
tags: { nodeType: node.type },
});
}

return nodeType.poll.call(pollFunctions);
Expand All @@ -1195,9 +1207,13 @@ export class Workflow {
): Promise<IWebhookResponseData> {
const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
if (nodeType === undefined) {
throw new Error(`The type of the webhook node "${node.name}" is not known.`);
throw new ApplicationError('Unknown node type of webhook node', {
extra: { nodeName: node.name },
});
} else if (nodeType.webhook === undefined) {
throw new Error(`The node "${node.name}" does not have any webhooks defined.`);
throw new ApplicationError('Node does not have any webhooks defined', {
extra: { nodeName: node.name },
});
}

const context = nodeExecuteFunctions.getExecuteWebhookFunctions(
Expand Down Expand Up @@ -1241,7 +1257,9 @@ export class Workflow {

const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
if (nodeType === undefined) {
throw new Error(`Node type "${node.type}" is not known so can not run it!`);
throw new ApplicationError('Node type is unknown so cannot run it', {
tags: { nodeType: node.type },
});
}

let connectionInputData: INodeExecutionData[] = [];
Expand Down
11 changes: 7 additions & 4 deletions packages/workflow/src/WorkflowDataProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import type { Workflow } from './Workflow';
import { augmentArray, augmentObject } from './AugmentObject';
import { deepCopy } from './utils';
import { getGlobalState } from './GlobalState';
import { ApplicationError } from './errors/application.error';

export function isResourceLocatorValue(value: unknown): value is INodeParameterResourceLocator {
return Boolean(
Expand Down Expand Up @@ -190,7 +191,9 @@ export class WorkflowDataProxy {
if (name[0] === '&') {
const key = name.slice(1);
if (!that.siblingParameters.hasOwnProperty(key)) {
throw new Error(`Could not find sibling parameter "${key}" on node "${nodeName}"`);
throw new ApplicationError('Could not find sibling parameter on node', {
extra: { nodeName, parameter: key },
});
}
returnValue = that.siblingParameters[key];
} else {
Expand Down Expand Up @@ -300,8 +303,8 @@ export class WorkflowDataProxy {
const taskData = that.runExecutionData.resultData.runData[nodeName][runIndex].data!;

if (!taskData.main?.length || taskData.main[0] === null) {
// throw new Error(`No data found for item-index: "${itemIndex}"`);
throw new ExpressionError('No data found from "main" input.', {
// throw new ApplicationError('No data found for item-index', { extra: { itemIndex } });
throw new ExpressionError('No data found from `main` input', {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
});
Expand Down Expand Up @@ -765,7 +768,7 @@ export class WorkflowDataProxy {
if (itemInput >= taskData.source.length) {
// `Could not resolve pairedItem as the defined node input '${itemInput}' does not exist on node '${sourceData!.previousNode}'.`
// Actual error does not matter as it gets caught below and `null` will be returned
throw new Error('Not found');
throw new ApplicationError('Not found');
}

return getPairedItem(destinationNodeName, taskData.source[itemInput], item);
Expand Down
3 changes: 2 additions & 1 deletion packages/workflow/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import FormData from 'form-data';
import type { BinaryFileType, JsonObject } from './Interfaces';
import { ApplicationError } from './errors/application.error';

const readStreamClasses = new Set(['ReadStream', 'Readable', 'ReadableStream']);

Expand Down Expand Up @@ -77,7 +78,7 @@ export const jsonParse = <T>(jsonString: string, options?: JSONParseOptions<T>):
if (options?.fallbackValue !== undefined) {
return options.fallbackValue;
} else if (options?.errorMessage) {
throw new Error(options.errorMessage);
throw new ApplicationError(options.errorMessage);
}

throw error;
Expand Down
Loading

0 comments on commit 67702c2

Please sign in to comment.