Skip to content

Commit

Permalink
refactor(core): Skip sending webhook activation errors to Sentry (no-…
Browse files Browse the repository at this point in the history
…changelog) (#7171)
  • Loading branch information
netroy authored Sep 27, 2023
1 parent 07d072c commit ebce6fe
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 16 deletions.
6 changes: 2 additions & 4 deletions packages/cli/src/ActiveWebhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
WorkflowActivateMode,
WorkflowExecuteMode,
} from 'n8n-workflow';

import { WebhookPathAlreadyTakenError } from 'n8n-workflow';
import * as NodeExecuteFunctions from 'n8n-core';

@Service()
Expand Down Expand Up @@ -46,9 +46,7 @@ export class ActiveWebhooks {

// check that there is not a webhook already registered with that path/method
if (this.webhookUrls[webhookKey] && !webhookData.webhookId) {
throw new Error(
`The URL path that the "${webhookData.node}" node uses is already taken. Please change it to something else.`,
);
throw new WebhookPathAlreadyTakenError(webhookData.node);
}

if (this.workflowWebhooks[webhookData.workflowId] === undefined) {
Expand Down
8 changes: 3 additions & 5 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
WorkflowActivationError,
LoggerProxy as Logger,
ErrorReporterProxy as ErrorReporter,
WebhookPathAlreadyTakenError,
} from 'n8n-workflow';

import type express from 'express';
Expand Down Expand Up @@ -428,11 +429,8 @@ export class ActiveWorkflowRunner implements IWebhookManager {
// if it's a workflow from the the insert
// TODO check if there is standard error code for duplicate key violation that works
// with all databases
if (error.name === 'QueryFailedError') {
error = new Error(
`The URL path that the "${webhook.node}" node uses is already taken. Please change it to something else.`,
{ cause: error },
);
if (error instanceof Error && error.name === 'QueryFailedError') {
error = new WebhookPathAlreadyTakenError(webhook.node, error);
} else if (error.detail) {
// it's a error running the webhook methods (checkExists, create)
error.message = error.detail;
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/ErrorReporting.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createHash } from 'crypto';
import config from '@/config';
import { ErrorReporterProxy, NodeError } from 'n8n-workflow';
import { ErrorReporterProxy, ExecutionBaseError } from 'n8n-workflow';

let initialized = false;

Expand Down Expand Up @@ -39,7 +39,7 @@ export const initErrorHandling = async () => {

const seenErrors = new Set<string>();
addGlobalEventProcessor((event, { originalException }) => {
if (originalException instanceof NodeError && originalException.severity === 'warning')
if (originalException instanceof ExecutionBaseError && originalException.severity === 'warning')
return null;
if (!event.exception) return null;
const eventHash = createHash('sha1').update(JSON.stringify(event.exception)).digest('base64');
Expand Down
6 changes: 3 additions & 3 deletions packages/workflow/src/NodeErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ const STATUS_CODE_MESSAGES: IStatusCodeMessages = {
const UNKNOWN_ERROR_MESSAGE = 'UNKNOWN ERROR - check the detailed error for more information';
const UNKNOWN_ERROR_MESSAGE_CRED = 'UNKNOWN ERROR';

type Severity = 'warning' | 'error';
export type Severity = 'warning' | 'error';

interface ExecutionBaseErrorOptions {
cause?: Error | JsonObject;
Expand Down Expand Up @@ -136,6 +136,8 @@ export abstract class ExecutionBaseError extends Error {

lineNumber: number | undefined;

severity: Severity = 'error';

constructor(message: string, { cause }: ExecutionBaseErrorOptions) {
const options = cause instanceof Error ? { cause } : {};
super(message, options);
Expand Down Expand Up @@ -171,8 +173,6 @@ export abstract class ExecutionBaseError extends Error {
export abstract class NodeError extends ExecutionBaseError {
node: INode;

severity: Severity = 'error';

constructor(node: INode, error: Error | JsonObject) {
const message = error instanceof Error ? error.message : '';
super(message, { cause: error });
Expand Down
15 changes: 13 additions & 2 deletions packages/workflow/src/WorkflowActivationError.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { INode } from './Interfaces';
import { ExecutionBaseError } from './NodeErrors';
import { ExecutionBaseError, type Severity } from './NodeErrors';

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

/**
Expand All @@ -12,7 +13,7 @@ interface WorkflowActivationErrorOptions {
export class WorkflowActivationError extends ExecutionBaseError {
node: INode | undefined;

constructor(message: string, { cause, node }: WorkflowActivationErrorOptions) {
constructor(message: string, { cause, node, severity }: WorkflowActivationErrorOptions) {
let error = cause as Error;
if (cause instanceof ExecutionBaseError) {
error = new Error(cause.message);
Expand All @@ -23,5 +24,15 @@ export class WorkflowActivationError extends ExecutionBaseError {
super(message, { cause: error });
this.node = node;
this.message = message;
if (severity) this.severity = severity;
}
}

export class WebhookPathAlreadyTakenError 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 },
);
}
}

0 comments on commit ebce6fe

Please sign in to comment.