fix(Postgres Trigger Node): closeFunction errors should not prevent…

… a workflow from being deactivated (#8738)
n8n-io · Mar 5, 2024 · 7535203 · 7535203
1 parent 177d70e
commit 7535203
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 25 deletions.
diff --git a/packages/core/src/ActiveWorkflows.ts b/packages/core/src/ActiveWorkflows.ts
@@ -15,8 +15,10 @@ import type {
 } from 'n8n-workflow';
 import {
 	ApplicationError,
+	ErrorReporterProxy as ErrorReporter,
 	LoggerProxy as Logger,
 	toCronExpression,
+	TriggerCloseError,
 	WorkflowActivationError,
 	WorkflowDeactivationError,
 } from 'n8n-workflow';
@@ -238,6 +240,14 @@ export class ActiveWorkflows {
 		try {
 			await response.closeFunction();
 		} catch (e) {
+			if (e instanceof TriggerCloseError) {
+				Logger.error(
+					`There was a problem calling "closeFunction" on "${e.node.name}" in workflow "${workflowId}"`,
+				);
+				ErrorReporter.error(e, { extra: { target, workflowId } });
+				return;
+			}
+
 			const error = e instanceof Error ? e : new Error(`${e}`);
 
 			throw new WorkflowDeactivationError(

diff --git a/packages/nodes-base/nodes/Postgres/PostgresTrigger.node.ts b/packages/nodes-base/nodes/Postgres/PostgresTrigger.node.ts
@@ -1,5 +1,5 @@
 import {
-	NodeOperationError,
+	TriggerCloseError,
 	type IDataObject,
 	type INodeType,
 	type INodeTypeDescription,
@@ -257,33 +257,42 @@ export class PostgresTrigger implements INodeType {
 
 		const cleanUpDb = async () => {
 			try {
-				await connection.none('UNLISTEN $1:name', [pgNames.channelName]);
-				if (triggerMode === 'createTrigger') {
-					const functionName = pgNames.functionName.includes('(')
-						? pgNames.functionName.split('(')[0]
-						: pgNames.functionName;
-					await connection.any('DROP FUNCTION IF EXISTS $1:name CASCADE', [functionName]);
+				try {
+					// check if the connection is healthy
+					await connection.query('SELECT 1');
+				} catch {
+					// connection already closed. Can't perform cleanup
+					// eslint-disable-next-line n8n-nodes-base/node-execute-block-wrong-error-thrown
+					throw new TriggerCloseError(this.getNode(), { level: 'warning' });
+				}
+
+				try {
+					await connection.none('UNLISTEN $1:name', [pgNames.channelName]);
+					if (triggerMode === 'createTrigger') {
+						const functionName = pgNames.functionName.includes('(')
+							? pgNames.functionName.split('(')[0]
+							: pgNames.functionName;
+						await connection.any('DROP FUNCTION IF EXISTS $1:name CASCADE', [functionName]);
 
-					const schema = this.getNodeParameter('schema', undefined, {
-						extractValue: true,
-					}) as string;
-					const table = this.getNodeParameter('tableName', undefined, {
-						extractValue: true,
-					}) as string;
+						const schema = this.getNodeParameter('schema', undefined, {
+							extractValue: true,
+						}) as string;
+						const table = this.getNodeParameter('tableName', undefined, {
+							extractValue: true,
+						}) as string;
 
-					await connection.any('DROP TRIGGER IF EXISTS $1:name ON $2:name.$3:name CASCADE', [
-						pgNames.triggerName,
-						schema,
-						table,
-					]);
+						await connection.any('DROP TRIGGER IF EXISTS $1:name ON $2:name.$3:name CASCADE', [
+							pgNames.triggerName,
+							schema,
+							table,
+						]);
+					}
+				} catch (error) {
+					// eslint-disable-next-line n8n-nodes-base/node-execute-block-wrong-error-thrown
+					throw new TriggerCloseError(this.getNode(), { cause: error as Error, level: 'error' });
 				}
-				connection.client.removeListener('notification', onNotification);
-			} catch (error) {
-				throw new NodeOperationError(
-					this.getNode(),
-					`Postgres Trigger Error: ${(error as Error).message}`,
-				);
 			} finally {
+				connection.client.removeListener('notification', onNotification);
 				if (!db.$pool.ending) await db.$pool.end();
 			}
 		};

diff --git a/packages/workflow/src/errors/application.error.ts b/packages/workflow/src/errors/application.error.ts
@@ -1,7 +1,7 @@
 import callsites from 'callsites';
 import type { Event } from '@sentry/node';
 
-type Level = 'warning' | 'error' | 'fatal' | 'info';
+export type Level = 'warning' | 'error' | 'fatal' | 'info';
 
 export type ReportingOptions = {
 	level?: Level;

diff --git a/packages/workflow/src/errors/index.ts b/packages/workflow/src/errors/index.ts
@@ -10,6 +10,7 @@ export { WorkflowDeactivationError } from './workflow-deactivation.error';
 export { WorkflowOperationError } from './workflow-operation.error';
 export { SubworkflowOperationError } from './subworkflow-operation.error';
 export { CliWorkflowOperationError } from './cli-subworkflow-operation.error';
+export { TriggerCloseError } from './trigger-close.error';
 
 export { NodeError } from './abstract/node.error';
 export { ExecutionBaseError } from './abstract/execution-base.error';

diff --git a/packages/workflow/src/errors/trigger-close.error.ts b/packages/workflow/src/errors/trigger-close.error.ts
@@ -0,0 +1,16 @@
+import type { INode } from '../Interfaces';
+import { ApplicationError, type Level } from './application.error';
+
+interface TriggerCloseErrorOptions extends ErrorOptions {
+	level: Level;
+}
+
+export class TriggerCloseError extends ApplicationError {
+	constructor(
+		readonly node: INode,
+		{ cause, level }: TriggerCloseErrorOptions,
+	) {
+		super('Trigger Close Failed', { cause, extra: { nodeName: node.name } });
+		this.level = level;
+	}
+}