refactor(core): Reorganize error hierarchy in core and workflow packages (no-changelog) (#7820)

Ensure all errors in `core` and `workflow` inherit from
`ApplicationError` so that we start normalizing all the errors we report
to Sentry

Follow-up to:
https://github.com/n8n-io/n8n/pull/7757#discussion_r1404338844

### `core` package

`ApplicationError`
- `FileSystemError` (abstract)
	- `FileNotFoundError`
	- `DisallowedFilepathError`
- `BinaryDataError` (abstract)
	- `InvalidModeError`
	- `InvalidManagerError`
- `InvalidExecutionMetadataError`

### `workflow` package

`ApplicationError`
- `ExecutionBaseError` (abstract)
	- `WorkflowActivationError`
		- `WorkflowDeactivationError`
		- `WebhookTakenError`
	- `WorkflowOperationError`
		- `SubworkflowOperationError`
			- `CliWorkflowOperationError`
	- `ExpressionError`
		- `ExpressionExtensionError`
	- `NodeError` (abstract)
		- `NodeOperationError`
		- `NodeApiError`
	- `NodeSSLError`

Up next:
- Reorganize errors in `cli`
- Flatten the hierarchy in `workflow` (do we really need
`ExecutionBaseError`?)
- Remove `ExecutionError` type
- Stop throwing plain `Error`s
- Replace `severity` with `level`
- Add node and credential types as `tags`
- Add workflow IDs and execution IDs as `extras`
This commit is contained in:
Iván Ovejero
2023-11-27 15:33:21 +01:00
committed by GitHub
parent aae45b043b
commit dff8456382
53 changed files with 758 additions and 694 deletions

View File

@@ -2,12 +2,13 @@ import { readFile, stat } from 'node:fs/promises';
import prettyBytes from 'pretty-bytes';
import Container, { Service } from 'typedi';
import { BINARY_ENCODING } from 'n8n-workflow';
import { UnknownManagerError, InvalidModeError } from './errors';
import { InvalidModeError } from '../errors/invalid-mode.error';
import { areConfigModes, toBuffer } from './utils';
import type { Readable } from 'stream';
import type { BinaryData } from './types';
import type { INodeExecutionData, IBinaryData } from 'n8n-workflow';
import { InvalidManagerError } from '../errors/invalid-manager.error';
@Service()
export class BinaryDataService {
@@ -241,6 +242,6 @@ export class BinaryDataService {
if (manager) return manager;
throw new UnknownManagerError(mode);
throw new InvalidManagerError(mode);
}
}

View File

@@ -4,10 +4,11 @@ import path from 'node:path';
import { v4 as uuid } from 'uuid';
import { jsonParse } from 'n8n-workflow';
import { assertDir, doesNotExist } from './utils';
import { BinaryFileNotFoundError, InvalidPathError } from '../errors';
import { DisallowedFilepathError } from '../errors/disallowed-filepath.error';
import type { Readable } from 'stream';
import type { BinaryData } from './types';
import { FileNotFoundError } from '../errors/file-not-found.error';
const EXECUTION_ID_EXTRACTOR =
/^(\w+)(?:[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12})$/;
@@ -47,7 +48,7 @@ export class FileSystemManager implements BinaryData.Manager {
const filePath = this.resolvePath(fileId);
if (await doesNotExist(filePath)) {
throw new BinaryFileNotFoundError(filePath);
throw new FileNotFoundError(filePath);
}
return createReadStream(filePath, { highWaterMark: chunkSize });
@@ -57,7 +58,7 @@ export class FileSystemManager implements BinaryData.Manager {
const filePath = this.resolvePath(fileId);
if (await doesNotExist(filePath)) {
throw new BinaryFileNotFoundError(filePath);
throw new FileNotFoundError(filePath);
}
return fs.readFile(filePath);
@@ -171,7 +172,7 @@ export class FileSystemManager implements BinaryData.Manager {
const returnPath = path.join(this.storagePath, ...args);
if (path.relative(this.storagePath, returnPath).startsWith('..')) {
throw new InvalidPathError(returnPath);
throw new DisallowedFilepathError(returnPath);
}
return returnPath;
@@ -190,7 +191,7 @@ export class FileSystemManager implements BinaryData.Manager {
const stats = await fs.stat(filePath);
return stats.size;
} catch (error) {
throw new BinaryFileNotFoundError(filePath);
throw new FileNotFoundError(filePath);
}
}
}

View File

@@ -1,11 +0,0 @@
import { CONFIG_MODES } from './utils';
export class InvalidModeError extends Error {
message = `Invalid binary data mode. Valid modes: ${CONFIG_MODES.join(', ')}`;
}
export class UnknownManagerError extends Error {
constructor(mode: string) {
super(`No binary data manager found for: ${mode}`);
}
}

View File

@@ -1,20 +1,9 @@
import type { IRunExecutionData } from 'n8n-workflow';
import { LoggerProxy as Logger } from 'n8n-workflow';
import { InvalidExecutionMetadataError } from './errors/invalid-execution-metadata.error';
export const KV_LIMIT = 10;
export class ExecutionMetadataValidationError extends Error {
constructor(
public type: 'key' | 'value',
key: unknown,
message?: string,
options?: ErrorOptions,
) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
super(message ?? `Custom data ${type}s must be a string (key "${key}")`, options);
}
}
export function setWorkflowExecutionMetadata(
executionData: IRunExecutionData,
key: string,
@@ -31,17 +20,17 @@ export function setWorkflowExecutionMetadata(
return;
}
if (typeof key !== 'string') {
throw new ExecutionMetadataValidationError('key', key);
throw new InvalidExecutionMetadataError('key', key);
}
if (key.replace(/[A-Za-z0-9_]/g, '').length !== 0) {
throw new ExecutionMetadataValidationError(
throw new InvalidExecutionMetadataError(
'key',
key,
`Custom date key can only contain characters "A-Za-z0-9_" (key "${key}")`,
);
}
if (typeof value !== 'string' && typeof value !== 'number' && typeof value !== 'bigint') {
throw new ExecutionMetadataValidationError('value', key);
throw new InvalidExecutionMetadataError('value', key);
}
const val = String(value);
if (key.length > 50) {

View File

@@ -38,7 +38,6 @@ import type {
BinaryHelperFunctions,
ConnectionTypes,
ContextType,
ExecutionError,
FieldType,
FileSystemHelperFunctions,
FunctionsBase,
@@ -101,7 +100,7 @@ import {
NodeApiError,
NodeHelpers,
NodeOperationError,
NodeSSLError,
NodeSslError,
OAuth2GrantType,
WorkflowDataProxy,
createDeferredPromise,
@@ -143,7 +142,7 @@ import {
getWorkflowExecutionMetadata,
setAllWorkflowExecutionMetadata,
setWorkflowExecutionMetadata,
} from './WorkflowExecutionMetadata';
} from './ExecutionMetadata';
import { getSecretsProxy } from './Secrets';
import Container from 'typedi';
import type { BinaryData } from './BinaryData/types';
@@ -808,7 +807,7 @@ export async function proxyRequestToAxios(
response: pick(response, ['headers', 'status', 'statusText']),
});
} else if ('rejectUnauthorized' in configObject && error.code?.includes('CERT')) {
throw new NodeSSLError(error);
throw new NodeSslError(error);
}
}
@@ -3442,7 +3441,7 @@ export function getExecuteFunctions(
addInputData(
connectionType: ConnectionTypes,
data: INodeExecutionData[][] | ExecutionError,
data: INodeExecutionData[][] | ExecutionBaseError,
): { index: number } {
const nodeName = this.getNode().name;
let currentNodeRunIndex = 0;
@@ -3473,7 +3472,7 @@ export function getExecuteFunctions(
addOutputData(
connectionType: ConnectionTypes,
currentNodeRunIndex: number,
data: INodeExecutionData[][] | ExecutionError,
data: INodeExecutionData[][] | ExecutionBaseError,
): void {
addExecutionDataFunctions(
'output',

View File

@@ -6,7 +6,7 @@ import { setMaxListeners } from 'events';
import PCancelable from 'p-cancelable';
import type {
ExecutionError,
ExecutionBaseError,
ExecutionStatus,
GenericValue,
IConnection,
@@ -797,7 +797,7 @@ export class WorkflowExecute {
// Variables which hold temporary data for each node-execution
let executionData: IExecuteData;
let executionError: ExecutionError | undefined;
let executionError: ExecutionBaseError | undefined;
let executionNode: INode;
let nodeSuccessData: INodeExecutionData[][] | null | undefined;
let runIndex: number;
@@ -838,11 +838,13 @@ export class WorkflowExecute {
await this.executeHook('workflowExecuteBefore', [workflow]);
}
} catch (error) {
const e = error as unknown as ExecutionBaseError;
// Set the error that it can be saved correctly
executionError = {
...(error as NodeOperationError | NodeApiError),
message: (error as NodeOperationError | NodeApiError).message,
stack: (error as NodeOperationError | NodeApiError).stack,
...e,
message: e.message,
stack: e.stack,
};
// Set the incoming data of the node that it can be saved correctly
@@ -1253,10 +1255,12 @@ export class WorkflowExecute {
} catch (error) {
this.runExecutionData.resultData.lastNodeExecuted = executionData.node.name;
const e = error as unknown as ExecutionBaseError;
executionError = {
...(error as NodeOperationError | NodeApiError),
message: (error as NodeOperationError | NodeApiError).message,
stack: (error as NodeOperationError | NodeApiError).stack,
...e,
message: e.message,
stack: e.stack,
};
Logger.debug(`Running node "${executionNode.name}" finished with error`, {
@@ -1325,11 +1329,17 @@ export class WorkflowExecute {
lineResult.json.$error !== undefined &&
lineResult.json.$json !== undefined
) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
lineResult.error = lineResult.json.$error as NodeApiError | NodeOperationError;
lineResult.json = {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
error: (lineResult.json.$error as NodeApiError | NodeOperationError).message,
};
} else if (lineResult.error !== undefined) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
lineResult.json = { error: lineResult.error.message };
}
}
@@ -1697,7 +1707,7 @@ export class WorkflowExecute {
async processSuccessExecution(
startedAt: Date,
workflow: Workflow,
executionError?: ExecutionError,
executionError?: ExecutionBaseError,
closeFunction?: Promise<void>,
): Promise<IRun> {
const fullRunData = this.getFullRunData(startedAt);
@@ -1711,7 +1721,7 @@ export class WorkflowExecute {
...executionError,
message: executionError.message,
stack: executionError.stack,
} as ExecutionError;
} as ExecutionBaseError;
if (executionError.message?.includes('canceled')) {
fullRunData.status = 'canceled';
}

View File

@@ -0,0 +1,3 @@
import { ApplicationError } from 'n8n-workflow';
export abstract class BinaryDataError extends ApplicationError {}

View File

@@ -0,0 +1,7 @@
import { ApplicationError } from 'n8n-workflow';
export abstract class FileSystemError extends ApplicationError {
constructor(message: string, filePath: string) {
super(message, { extra: { filePath } });
}
}

View File

@@ -0,0 +1,7 @@
import { FileSystemError } from './abstract/filesystem.error';
export class DisallowedFilepathError extends FileSystemError {
constructor(filePath: string) {
super('Disallowed path detected', filePath);
}
}

View File

@@ -0,0 +1,7 @@
import { FileSystemError } from './abstract/filesystem.error';
export class FileNotFoundError extends FileSystemError {
constructor(filePath: string) {
super('File not found', filePath);
}
}

View File

@@ -1,21 +0,0 @@
import { ReportableError } from 'n8n-workflow';
abstract class FileSystemError extends ReportableError {
constructor(message: string, filePath: string) {
super(message, { extra: { filePath } });
}
}
export class FileNotFoundError extends FileSystemError {
constructor(filePath: string) {
super('File not found', filePath);
}
}
export class BinaryFileNotFoundError extends FileNotFoundError {}
export class InvalidPathError extends FileSystemError {
constructor(filePath: string) {
super('Invalid path detected', filePath);
}
}

View File

@@ -1 +1,5 @@
export { BinaryFileNotFoundError, FileNotFoundError, InvalidPathError } from './filesystem.errors';
export { FileNotFoundError } from './file-not-found.error';
export { DisallowedFilepathError } from './disallowed-filepath.error';
export { InvalidModeError } from './invalid-mode.error';
export { InvalidManagerError } from './invalid-manager.error';
export { InvalidExecutionMetadataError } from './invalid-execution-metadata.error';

View File

@@ -0,0 +1,13 @@
import { ApplicationError } from 'n8n-workflow';
export class InvalidExecutionMetadataError extends ApplicationError {
constructor(
public type: 'key' | 'value',
key: unknown,
message?: string,
options?: ErrorOptions,
) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
super(message ?? `Custom data ${type}s must be a string (key "${key}")`, options);
}
}

View File

@@ -0,0 +1,7 @@
import { BinaryDataError } from './abstract/binary-data.error';
export class InvalidManagerError extends BinaryDataError {
constructor(mode: string) {
super(`No binary data manager found for: ${mode}`);
}
}

View File

@@ -0,0 +1,8 @@
import { ApplicationError } from 'n8n-workflow';
import { CONFIG_MODES } from '../BinaryData/utils';
export class InvalidModeError extends ApplicationError {
constructor() {
super(`Invalid binary data mode. Valid modes: ${CONFIG_MODES.join(', ')}`);
}
}

View File

@@ -18,3 +18,4 @@ export * from './errors';
export { ObjectStoreService } from './ObjectStore/ObjectStore.service.ee';
export { BinaryData } from './BinaryData/types';
export { isStoredMode as isValidNonDefaultMode } from './BinaryData/utils';
export * from './ExecutionMetadata';