refactor(core): Extend error hierarchy (#12267)

This commit is contained in:
Tomi Turtiainen 2025-02-18 17:47:11 +02:00 committed by GitHub
parent 1e1f528466
commit 2ab59d775b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 293 additions and 28 deletions

View file

@ -2,7 +2,7 @@ import { QueryFailedError } from '@n8n/typeorm';
import type { ErrorEvent } from '@sentry/types'; import type { ErrorEvent } from '@sentry/types';
import { AxiosError } from 'axios'; import { AxiosError } from 'axios';
import { mock } from 'jest-mock-extended'; import { mock } from 'jest-mock-extended';
import { ApplicationError } from 'n8n-workflow'; import { ApplicationError, BaseError } from 'n8n-workflow';
import type { Logger } from '@/logging/logger'; import type { Logger } from '@/logging/logger';
@ -133,6 +133,43 @@ describe('ErrorReporter', () => {
expect(beforeSendFilter).toHaveBeenCalledWith(event, hint); expect(beforeSendFilter).toHaveBeenCalledWith(event, hint);
}); });
}); });
describe('BaseError', () => {
class TestError extends BaseError {}
it('should drop errors with shouldReport false', async () => {
const originalException = new TestError('test', { shouldReport: false });
expect(await errorReporter.beforeSend(event, { originalException })).toEqual(null);
});
it('should keep events with shouldReport true', async () => {
const originalException = new TestError('test', { shouldReport: true });
expect(await errorReporter.beforeSend(event, { originalException })).toEqual(event);
});
it('should set level, extra, and tags from BaseError', async () => {
const originalException = new TestError('Test error', {
level: 'error',
extra: { foo: 'bar' },
tags: { tag1: 'value1' },
});
const testEvent = {} as ErrorEvent;
const result = await errorReporter.beforeSend(testEvent, { originalException });
expect(result).toEqual({
level: 'error',
extra: { foo: 'bar' },
tags: {
packageName: 'core',
tag1: 'value1',
},
});
});
});
}); });
describe('error', () => { describe('error', () => {

View file

@ -3,7 +3,8 @@ import type { NodeOptions } from '@sentry/node';
import { close } from '@sentry/node'; import { close } from '@sentry/node';
import type { ErrorEvent, EventHint } from '@sentry/types'; import type { ErrorEvent, EventHint } from '@sentry/types';
import { AxiosError } from 'axios'; import { AxiosError } from 'axios';
import { ApplicationError, ExecutionCancelledError, type ReportingOptions } from 'n8n-workflow'; import type { ReportingOptions } from 'n8n-workflow';
import { ApplicationError, ExecutionCancelledError, BaseError } from 'n8n-workflow';
import { createHash } from 'node:crypto'; import { createHash } from 'node:crypto';
import type { InstanceType } from '@/instance-settings'; import type { InstanceType } from '@/instance-settings';
@ -44,11 +45,15 @@ export class ErrorReporter {
const context = executionId ? ` (execution ${executionId})` : ''; const context = executionId ? ` (execution ${executionId})` : '';
do { do {
const msg = [ let stack = '';
e.message + context, let meta = undefined;
e instanceof ApplicationError && e.level === 'error' && e.stack ? `\n${e.stack}\n` : '', if (e instanceof ApplicationError || e instanceof BaseError) {
].join(''); if (e.level === 'error' && e.stack) {
const meta = e instanceof ApplicationError ? e.extra : undefined; stack = `\n${e.stack}\n`;
}
meta = e.extra;
}
const msg = [e.message + context, stack].join('');
this.logger.error(msg, meta); this.logger.error(msg, meta);
e = e.cause as Error; e = e.cause as Error;
} while (e); } while (e);
@ -137,11 +142,17 @@ export class ErrorReporter {
if (originalException instanceof AxiosError) return null; if (originalException instanceof AxiosError) return null;
if (originalException instanceof BaseError) {
if (!originalException.shouldReport) return null;
this.extractEventDetailsFromN8nError(event, originalException);
}
if (this.isIgnoredSqliteError(originalException)) return null; if (this.isIgnoredSqliteError(originalException)) return null;
if (this.isApplicationError(originalException)) { if (originalException instanceof ApplicationError) {
if (this.isIgnoredApplicationError(originalException)) return null; if (this.isIgnoredApplicationError(originalException)) return null;
this.extractEventDetailsFromApplicationError(event, originalException); this.extractEventDetailsFromN8nError(event, originalException);
} }
if ( if (
@ -193,17 +204,13 @@ export class ErrorReporter {
); );
} }
private isApplicationError(error: unknown): error is ApplicationError {
return error instanceof ApplicationError;
}
private isIgnoredApplicationError(error: ApplicationError) { private isIgnoredApplicationError(error: ApplicationError) {
return error.level === 'warning'; return error.level === 'warning';
} }
private extractEventDetailsFromApplicationError( private extractEventDetailsFromN8nError(
event: ErrorEvent, event: ErrorEvent,
originalException: ApplicationError, originalException: ApplicationError | BaseError,
) { ) {
const { level, extra, tags } = originalException; const { level, extra, tags } = originalException;
event.level = level; event.level = level;

View file

@ -1,5 +1,6 @@
import type { Functionality, IDataObject, JsonObject } from '../../Interfaces'; import type { Functionality, IDataObject, JsonObject } from '../../Interfaces';
import { ApplicationError, type ReportingOptions } from '../application.error'; import { ApplicationError } from '../application.error';
import type { ReportingOptions } from '../error.types';
interface ExecutionBaseErrorOptions extends ReportingOptions { interface ExecutionBaseErrorOptions extends ReportingOptions {
cause?: Error; cause?: Error;

View file

@ -1,15 +1,13 @@
import type { Event } from '@sentry/node'; import type { Event } from '@sentry/node';
import callsites from 'callsites'; import callsites from 'callsites';
export type Level = 'warning' | 'error' | 'fatal' | 'info'; import type { ErrorLevel, ReportingOptions } from '@/errors/error.types';
export type ReportingOptions = {
level?: Level;
executionId?: string;
} & Pick<Event, 'tags' | 'extra'>;
/**
* @deprecated Use `UserError`, `OperationalError` or `UnexpectedError` instead.
*/
export class ApplicationError extends Error { export class ApplicationError extends Error {
level: Level; level: ErrorLevel;
readonly tags: NonNullable<Event['tags']>; readonly tags: NonNullable<Event['tags']>;

View file

@ -0,0 +1,58 @@
import type { Event } from '@sentry/node';
import callsites from 'callsites';
import type { ErrorTags, ErrorLevel, ReportingOptions } from '../error.types';
export type BaseErrorOptions = { description?: undefined | null } & ErrorOptions & ReportingOptions;
/**
* Base class for all errors
*/
export abstract class BaseError extends Error {
/**
* Error level. Defines which level the error should be logged/reported
* @default 'error'
*/
readonly level: ErrorLevel;
/**
* Whether the error should be reported to Sentry.
* @default true
*/
readonly shouldReport: boolean;
readonly description: string | null | undefined;
readonly tags: ErrorTags;
readonly extra?: Event['extra'];
readonly packageName?: string;
constructor(
message: string,
{
level = 'error',
description,
shouldReport,
tags = {},
extra,
...rest
}: BaseErrorOptions = {},
) {
super(message, rest);
this.level = level;
this.shouldReport = shouldReport ?? (level === 'error' || level === 'fatal');
this.description = description;
this.tags = tags;
this.extra = extra;
try {
const filePath = callsites()[2].getFileName() ?? '';
const match = /packages\/([^\/]+)\//.exec(filePath)?.[1];
if (match) this.tags.packageName = match;
} catch {}
}
}

View file

@ -0,0 +1,21 @@
import type { BaseErrorOptions } from './base.error';
import { BaseError } from './base.error';
export type OperationalErrorOptions = Omit<BaseErrorOptions, 'level'> & {
level?: 'info' | 'warning' | 'error';
};
/**
* Error that indicates a transient issue, like a network request failing,
* a database query timing out, etc. These are expected to happen, are
* transient by nature and should be handled gracefully.
*
* Default level: warning
*/
export class OperationalError extends BaseError {
constructor(message: string, opts: OperationalErrorOptions = {}) {
opts.level = opts.level ?? 'warning';
super(message, opts);
}
}

View file

@ -0,0 +1,21 @@
import type { BaseErrorOptions } from './base.error';
import { BaseError } from './base.error';
export type UnexpectedErrorOptions = Omit<BaseErrorOptions, 'level'> & {
level?: 'error' | 'fatal';
};
/**
* Error that indicates something is wrong in the code: logic mistakes,
* unhandled cases, assertions that fail. These are not recoverable and
* should be brought to developers' attention.
*
* Default level: error
*/
export class UnexpectedError extends BaseError {
constructor(message: string, opts: UnexpectedErrorOptions = {}) {
opts.level = opts.level ?? 'error';
super(message, opts);
}
}

View file

@ -0,0 +1,24 @@
import type { BaseErrorOptions } from './base.error';
import { BaseError } from './base.error';
export type UserErrorOptions = Omit<BaseErrorOptions, 'level'> & {
level?: 'info' | 'warning';
description?: string | null | undefined;
};
/**
* Error that indicates the user performed an action that caused an error.
* E.g. provided invalid input, tried to access a resource theyre not
* authorized to, or violates a business rule.
*
* Default level: info
*/
export class UserError extends BaseError {
readonly description: string | null | undefined;
constructor(message: string, opts: UserErrorOptions = {}) {
opts.level = opts.level ?? 'info';
super(message, opts);
}
}

View file

@ -0,0 +1,14 @@
import type { Event } from '@sentry/node';
export type ErrorLevel = 'fatal' | 'error' | 'warning' | 'info';
export type ErrorTags = NonNullable<Event['tags']>;
export type ReportingOptions = {
/** Whether the error should be reported to Sentry */
shouldReport?: boolean;
level?: ErrorLevel;
tags?: ErrorTags;
extra?: Event['extra'];
executionId?: string;
};

View file

@ -1,4 +1,9 @@
export { ApplicationError, type ReportingOptions } from './application.error'; export type * from './error.types';
export { BaseError, type BaseErrorOptions } from './base/base.error';
export { OperationalError, type OperationalErrorOptions } from './base/operational.error';
export { UnexpectedError, type UnexpectedErrorOptions } from './base/unexpected.error';
export { UserError, type UserErrorOptions } from './base/user.error';
export { ApplicationError } from './application.error';
export { ExpressionError } from './expression.error'; export { ExpressionError } from './expression.error';
export { CredentialAccessError } from './credential-access-error'; export { CredentialAccessError } from './credential-access-error';
export { ExecutionCancelledError } from './execution-cancelled.error'; export { ExecutionCancelledError } from './execution-cancelled.error';

View file

@ -5,7 +5,7 @@ import { AxiosError } from 'axios';
import { parseString } from 'xml2js'; import { parseString } from 'xml2js';
import { NodeError } from './abstract/node.error'; import { NodeError } from './abstract/node.error';
import type { ReportingOptions } from './application.error'; import type { ErrorLevel } from './error.types';
import { import {
NO_OP_NODE_TYPE, NO_OP_NODE_TYPE,
UNKNOWN_ERROR_DESCRIPTION, UNKNOWN_ERROR_DESCRIPTION,
@ -27,7 +27,7 @@ export interface NodeOperationErrorOptions {
description?: string; description?: string;
runIndex?: number; runIndex?: number;
itemIndex?: number; itemIndex?: number;
level?: ReportingOptions['level']; level?: ErrorLevel;
messageMapping?: { [key: string]: string }; // allows to pass custom mapping for error messages scoped to a node messageMapping?: { [key: string]: string }; // allows to pass custom mapping for error messages scoped to a node
functionality?: Functionality; functionality?: Functionality;
type?: string; type?: string;

View file

@ -1,8 +1,9 @@
import { ApplicationError, type Level } from './application.error'; import { ApplicationError } from './application.error';
import type { ErrorLevel } from './error.types';
import type { INode } from '../Interfaces'; import type { INode } from '../Interfaces';
interface TriggerCloseErrorOptions extends ErrorOptions { interface TriggerCloseErrorOptions extends ErrorOptions {
level: Level; level: ErrorLevel;
} }
export class TriggerCloseError extends ApplicationError { export class TriggerCloseError extends ApplicationError {

View file

@ -0,0 +1,26 @@
import { BaseError } from '@/errors/base/base.error';
import { OperationalError } from '@/errors/base/operational.error';
describe('OperationalError', () => {
it('should be an instance of OperationalError', () => {
const error = new OperationalError('test');
expect(error).toBeInstanceOf(OperationalError);
});
it('should be an instance of BaseError', () => {
const error = new OperationalError('test');
expect(error).toBeInstanceOf(BaseError);
});
it('should have correct defaults', () => {
const error = new OperationalError('test');
expect(error.level).toBe('warning');
expect(error.shouldReport).toBe(false);
});
it('should allow overriding the default level and shouldReport', () => {
const error = new OperationalError('test', { level: 'error', shouldReport: true });
expect(error.level).toBe('error');
expect(error.shouldReport).toBe(true);
});
});

View file

@ -0,0 +1,26 @@
import { BaseError } from '@/errors/base/base.error';
import { UnexpectedError } from '@/errors/base/unexpected.error';
describe('UnexpectedError', () => {
it('should be an instance of UnexpectedError', () => {
const error = new UnexpectedError('test');
expect(error).toBeInstanceOf(UnexpectedError);
});
it('should be an instance of BaseError', () => {
const error = new UnexpectedError('test');
expect(error).toBeInstanceOf(BaseError);
});
it('should have correct defaults', () => {
const error = new UnexpectedError('test');
expect(error.level).toBe('error');
expect(error.shouldReport).toBe(true);
});
it('should allow overriding the default level and shouldReport', () => {
const error = new UnexpectedError('test', { level: 'fatal', shouldReport: false });
expect(error.level).toBe('fatal');
expect(error.shouldReport).toBe(false);
});
});

View file

@ -0,0 +1,26 @@
import { BaseError } from '@/errors/base/base.error';
import { UserError } from '@/errors/base/user.error';
describe('UserError', () => {
it('should be an instance of UserError', () => {
const error = new UserError('test');
expect(error).toBeInstanceOf(UserError);
});
it('should be an instance of BaseError', () => {
const error = new UserError('test');
expect(error).toBeInstanceOf(BaseError);
});
it('should have correct defaults', () => {
const error = new UserError('test');
expect(error.level).toBe('info');
expect(error.shouldReport).toBe(false);
});
it('should allow overriding the default level and shouldReport', () => {
const error = new UserError('test', { level: 'warning', shouldReport: true });
expect(error.level).toBe('warning');
expect(error.shouldReport).toBe(true);
});
});