From 1d46983b24cd8ecae3f92c44bdc62001261797fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 7 Dec 2023 16:57:02 +0100 Subject: [PATCH] refactor: Unify `severity` and `level` for all application errors for Sentry (no-changelog) (#7956) ## Summary Unify `severity` and `level` for all backend application errors for Sentry Follow-up to: https://github.com/n8n-io/n8n/pull/7914#issuecomment-1840433542 ... #### How to test the change: 1. ... ## Issues fixed Include links to Github issue or Community forum post or **Linear ticket**: > Important in order to close automatically and provide context to reviewers ... ## Review / Merge checklist - [ ] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests. > > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e). --- packages/cli/src/ErrorReporting.ts | 5 +---- .../src/UserManagement/PermissionChecker.ts | 4 ++-- .../src/errors/credential-not-found.error.ts | 6 ++---- packages/core/src/NodeExecuteFunctions.ts | 18 +++++++++--------- .../nodes/Airtable/v2/methods/listSearch.ts | 2 +- .../nodes/Airtable/v2/methods/loadOptions.ts | 4 ++-- .../Airtable/v2/methods/resourceMapping.ts | 2 +- .../nodes/Facebook/FacebookTrigger.node.ts | 2 +- .../FacebookLeadAdsTrigger.node.ts | 2 +- .../nodes/Github/GithubTrigger.node.ts | 4 ++-- .../Google/Sheet/v2/helpers/GoogleSheet.ts | 2 +- .../Sheet/v2/helpers/GoogleSheets.utils.ts | 2 +- .../nodes/Slack/V2/GenericFunctions.ts | 4 ++-- packages/workflow/src/Interfaces.ts | 1 - .../errors/abstract/execution-base.error.ts | 4 +--- .../workflow/src/errors/application.error.ts | 2 +- packages/workflow/src/errors/node-api.error.ts | 12 ++++++------ .../src/errors/node-operation.error.ts | 2 +- .../workflow/src/errors/webhook-taken.error.ts | 2 +- .../src/errors/workflow-activation.error.ts | 9 +++++---- .../src/errors/workflow-operation.error.ts | 2 +- 21 files changed, 42 insertions(+), 49 deletions(-) diff --git a/packages/cli/src/ErrorReporting.ts b/packages/cli/src/ErrorReporting.ts index 7733f81665..f59b29c13c 100644 --- a/packages/cli/src/ErrorReporting.ts +++ b/packages/cli/src/ErrorReporting.ts @@ -1,6 +1,6 @@ import { createHash } from 'crypto'; import config from '@/config'; -import { ErrorReporterProxy, ApplicationError, ExecutionBaseError } from 'n8n-workflow'; +import { ErrorReporterProxy, ApplicationError } from 'n8n-workflow'; let initialized = false; @@ -39,9 +39,6 @@ export const initErrorHandling = async () => { const seenErrors = new Set(); addGlobalEventProcessor((event, { originalException }) => { - if (originalException instanceof ExecutionBaseError && originalException.severity === 'warning') - return null; - if (originalException instanceof ApplicationError) { const { level, extra } = originalException; if (level === 'warning') return null; diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 514664814e..6fb85281e8 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -72,7 +72,7 @@ export class PermissionChecker { throw new NodeOperationError(nodeToFlag, 'Node has no access to credential', { description: 'Please recreate the credential or ask its owner to share it with you.', - severity: 'warning', + level: 'warning', }); } @@ -151,7 +151,7 @@ export class PermissionChecker { if (!cred.id) { throw new NodeOperationError(node, 'Node uses invalid credential', { description: 'Please recreate the credential.', - severity: 'warning', + level: 'warning', }); } diff --git a/packages/cli/src/errors/credential-not-found.error.ts b/packages/cli/src/errors/credential-not-found.error.ts index 3826dc70bd..ac47a0eaea 100644 --- a/packages/cli/src/errors/credential-not-found.error.ts +++ b/packages/cli/src/errors/credential-not-found.error.ts @@ -1,10 +1,8 @@ -import { ApplicationError, type Severity } from 'n8n-workflow'; +import { ApplicationError } from 'n8n-workflow'; export class CredentialNotFoundError extends ApplicationError { - severity: Severity; - constructor(credentialId: string, credentialType: string) { super(`Credential with ID "${credentialId}" does not exist for type "${credentialType}".`); - this.severity = 'warning'; + this.level = 'warning'; } } diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index a251a5c38e..e0796d1fad 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -1528,7 +1528,7 @@ export async function httpRequestWithAuthentication( throw new NodeOperationError( node, `Node "${node.name}" does not have any credentials of type "${credentialsType}" set!`, - { severity: 'warning' }, + { level: 'warning' }, ); } @@ -1722,7 +1722,7 @@ export async function requestWithAuthentication( throw new NodeOperationError( node, `Node "${node.name}" does not have any credentials of type "${credentialsType}" set!`, - { severity: 'warning' }, + { level: 'warning' }, ); } @@ -1881,7 +1881,7 @@ export async function getCredentials( throw new NodeOperationError( node, `Node type "${node.type}" does not have any credentials defined!`, - { severity: 'warning' }, + { level: 'warning' }, ); } @@ -1892,7 +1892,7 @@ export async function getCredentials( throw new NodeOperationError( node, `Node type "${node.type}" does not have any credentials of type "${type}" defined!`, - { severity: 'warning' }, + { level: 'warning' }, ); } @@ -1917,14 +1917,14 @@ export async function getCredentials( // Credentials are required so error if (!node.credentials) { throw new NodeOperationError(node, 'Node does not have any credentials set!', { - severity: 'warning', + level: 'warning', }); } if (!node.credentials[type]) { throw new NodeOperationError( node, `Node does not have any credentials set for "${type}"!`, - { severity: 'warning' }, + { level: 'warning' }, ); } } else { @@ -2903,7 +2903,7 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => throw error.code === 'ENOENT' ? new NodeOperationError(node, error, { message: `The file "${String(filePath)}" could not be accessed.`, - severity: 'warning', + level: 'warning', }) : error; } @@ -2911,7 +2911,7 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => const allowedPaths = getAllowedPaths(); const message = allowedPaths.length ? ` Allowed paths: ${allowedPaths.join(', ')}` : ''; throw new NodeOperationError(node, `Access to the file is not allowed.${message}`, { - severity: 'warning', + level: 'warning', }); } return createReadStream(filePath); @@ -2924,7 +2924,7 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => async writeContentToFile(filePath, content, flag) { if (isFilePathBlocked(filePath as string)) { throw new NodeOperationError(node, `The file "${String(filePath)}" is not writable.`, { - severity: 'warning', + level: 'warning', }); } return fsWriteFile(filePath, content, { encoding: 'binary', flag }); diff --git a/packages/nodes-base/nodes/Airtable/v2/methods/listSearch.ts b/packages/nodes-base/nodes/Airtable/v2/methods/listSearch.ts index 38fe8b5c25..10f2d89c83 100644 --- a/packages/nodes-base/nodes/Airtable/v2/methods/listSearch.ts +++ b/packages/nodes-base/nodes/Airtable/v2/methods/listSearch.ts @@ -119,7 +119,7 @@ export async function viewSearch( if (!tableData) { throw new NodeOperationError(this.getNode(), 'Table information could not be found!', { - severity: 'warning', + level: 'warning', }); } diff --git a/packages/nodes-base/nodes/Airtable/v2/methods/loadOptions.ts b/packages/nodes-base/nodes/Airtable/v2/methods/loadOptions.ts index db395ec680..79aaabf1ef 100644 --- a/packages/nodes-base/nodes/Airtable/v2/methods/loadOptions.ts +++ b/packages/nodes-base/nodes/Airtable/v2/methods/loadOptions.ts @@ -21,7 +21,7 @@ export async function getColumns(this: ILoadOptionsFunctions): Promise; export class ApplicationError extends Error { - readonly level: Level; + level: Level; readonly tags?: Event['tags']; diff --git a/packages/workflow/src/errors/node-api.error.ts b/packages/workflow/src/errors/node-api.error.ts index ef57ff3e00..c1dc5c86b0 100644 --- a/packages/workflow/src/errors/node-api.error.ts +++ b/packages/workflow/src/errors/node-api.error.ts @@ -9,18 +9,18 @@ import type { JsonObject, IDataObject, IStatusCodeMessages, - Severity, Functionality, } from '../Interfaces'; import { NodeError } from './abstract/node.error'; import { removeCircularRefs } from '../utils'; +import type { ReportingOptions } from './application.error'; export interface NodeOperationErrorOptions { message?: string; description?: string; runIndex?: number; itemIndex?: number; - severity?: Severity; + level?: ReportingOptions['level']; messageMapping?: { [key: string]: string }; // allows to pass custom mapping for error messages scoped to a node functionality?: Functionality; } @@ -120,7 +120,7 @@ export class NodeApiError extends NodeError { parseXml, runIndex, itemIndex, - severity, + level, functionality, messageMapping, }: NodeApiErrorOptions = {}, @@ -174,10 +174,10 @@ export class NodeApiError extends NodeError { this.findProperty(errorResponse, ERROR_STATUS_PROPERTIES, ERROR_NESTING_PROPERTIES) ?? null; } - if (severity) { - this.severity = severity; + if (level) { + this.level = level; } else if (this.httpCode?.charAt(0) !== '5') { - this.severity = 'warning'; + this.level = 'warning'; } // set description of this error diff --git a/packages/workflow/src/errors/node-operation.error.ts b/packages/workflow/src/errors/node-operation.error.ts index 719673a913..2db531510a 100644 --- a/packages/workflow/src/errors/node-operation.error.ts +++ b/packages/workflow/src/errors/node-operation.error.ts @@ -19,7 +19,7 @@ export class NodeOperationError extends NodeError { super(node, error); if (options.message) this.message = options.message; - if (options.severity) this.severity = options.severity; + if (options.level) this.level = options.level; if (options.functionality) this.functionality = options.functionality; this.description = options.description; this.context.runIndex = options.runIndex; diff --git a/packages/workflow/src/errors/webhook-taken.error.ts b/packages/workflow/src/errors/webhook-taken.error.ts index c19ac4cb50..091094ade5 100644 --- a/packages/workflow/src/errors/webhook-taken.error.ts +++ b/packages/workflow/src/errors/webhook-taken.error.ts @@ -4,7 +4,7 @@ export class WebhookPathTakenError extends WorkflowActivationError { constructor(nodeName: string, cause?: Error) { super( `The URL path that the "${nodeName}" node uses is already taken. Please change it to something else.`, - { severity: 'warning', cause }, + { level: 'warning', cause }, ); } } diff --git a/packages/workflow/src/errors/workflow-activation.error.ts b/packages/workflow/src/errors/workflow-activation.error.ts index 3ce2a26636..16e68effda 100644 --- a/packages/workflow/src/errors/workflow-activation.error.ts +++ b/packages/workflow/src/errors/workflow-activation.error.ts @@ -1,10 +1,11 @@ -import type { INode, Severity } from '../Interfaces'; +import type { INode } from '../Interfaces'; import { ExecutionBaseError } from './abstract/execution-base.error'; +import type { ApplicationError } from './application.error'; interface WorkflowActivationErrorOptions { cause?: Error; node?: INode; - severity?: Severity; + level?: ApplicationError['level']; workflowId?: string; } @@ -18,7 +19,7 @@ export class WorkflowActivationError extends ExecutionBaseError { constructor( message: string, - { cause, node, severity, workflowId }: WorkflowActivationErrorOptions = {}, + { cause, node, level, workflowId }: WorkflowActivationErrorOptions = {}, ) { let error = cause as Error; if (cause instanceof ExecutionBaseError) { @@ -31,6 +32,6 @@ export class WorkflowActivationError extends ExecutionBaseError { this.node = node; this.workflowId = workflowId; this.message = message; - if (severity) this.severity = severity; + if (level) this.level = level; } } diff --git a/packages/workflow/src/errors/workflow-operation.error.ts b/packages/workflow/src/errors/workflow-operation.error.ts index 214364dc2f..25b47e149c 100644 --- a/packages/workflow/src/errors/workflow-operation.error.ts +++ b/packages/workflow/src/errors/workflow-operation.error.ts @@ -15,7 +15,7 @@ export class WorkflowOperationError extends ExecutionBaseError { constructor(message: string, node?: INode, description?: string) { super(message, { cause: undefined }); - this.severity = 'warning'; + this.level = 'warning'; this.name = this.constructor.name; if (description) this.description = description; this.node = node;