From e8e44f6b6ea482f9b3d06d427adc36938b406c05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 22 Sep 2023 11:48:20 +0200 Subject: [PATCH] refactor(core): Log binary data file write errors (no-changelog) (#7237) This PR adds logging for binary data file write errors, to capture why executions sometimes point to non-existing binary data files. See [Sentry error](https://n8nio.sentry.io/issues/4495134693/?alert_rule_id=14556563&alert_type=issue¬ification_uuid=4b50a5da-6ae9-472e-9658-984cca824762&project=4503924908883968&referrer=slack). --- .../core/src/BinaryDataManager/FileSystem.ts | 3 +- packages/core/src/BinaryDataManager/index.ts | 13 ++++++-- packages/core/src/LoadNodeDetails.ts | 6 ++-- packages/core/src/WorkflowExecute.ts | 9 ++++-- .../core/src/decorators/LogCatch.decorator.ts | 30 +++++++++++++++++++ packages/core/test/helpers/index.ts | 3 +- packages/core/tsconfig.json | 2 ++ 7 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/decorators/LogCatch.decorator.ts diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index 5c4bdd32a9..bcc40a38ec 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -6,7 +6,8 @@ import type { Readable } from 'stream'; import type { BinaryMetadata } from 'n8n-workflow'; import { jsonParse } from 'n8n-workflow'; -import type { IBinaryDataConfig, IBinaryDataManager } from '../Interfaces'; +import { IBinaryDataConfig } from '../Interfaces'; +import type { IBinaryDataManager } from '../Interfaces'; import { FileNotFoundError } from '../errors'; const executionExtractionRegexp = diff --git a/packages/core/src/BinaryDataManager/index.ts b/packages/core/src/BinaryDataManager/index.ts index eca32f184d..677a5a73f6 100644 --- a/packages/core/src/BinaryDataManager/index.ts +++ b/packages/core/src/BinaryDataManager/index.ts @@ -1,11 +1,13 @@ import { readFile, stat } from 'fs/promises'; -import type { BinaryMetadata, IBinaryData, INodeExecutionData } from 'n8n-workflow'; +import type { BinaryMetadata, INodeExecutionData } from 'n8n-workflow'; import prettyBytes from 'pretty-bytes'; import type { Readable } from 'stream'; -import { BINARY_ENCODING } from 'n8n-workflow'; -import type { IBinaryDataConfig, IBinaryDataManager } from '../Interfaces'; +import { BINARY_ENCODING, LoggerProxy as Logger, IBinaryData } from 'n8n-workflow'; +import { IBinaryDataConfig } from '../Interfaces'; +import type { IBinaryDataManager } from '../Interfaces'; import { BinaryDataFileSystem } from './FileSystem'; import { binaryToBuffer } from './utils'; +import { LogCatch } from '../decorators/LogCatch.decorator'; export class BinaryDataManager { static instance: BinaryDataManager | undefined; @@ -47,6 +49,7 @@ export class BinaryDataManager { return BinaryDataManager.instance; } + @LogCatch((error) => Logger.error('Failed to copy binary data file', { error })) async copyBinaryFile( binaryData: IBinaryData, filePath: string, @@ -79,6 +82,7 @@ export class BinaryDataManager { return binaryData; } + @LogCatch((error) => Logger.error('Failed to write binary data file', { error })) async storeBinaryData( binaryData: IBinaryData, input: Buffer | Readable, @@ -162,6 +166,9 @@ export class BinaryDataManager { } } + @LogCatch((error) => + Logger.error('Failed to copy all binary data files for execution', { error }), + ) async duplicateBinaryData( inputData: Array, executionId: string, diff --git a/packages/core/src/LoadNodeDetails.ts b/packages/core/src/LoadNodeDetails.ts index 12b807c7cc..a612759f07 100644 --- a/packages/core/src/LoadNodeDetails.ts +++ b/packages/core/src/LoadNodeDetails.ts @@ -1,11 +1,11 @@ -import type { - INode, +import type { INode } from 'n8n-workflow'; +import { + Workflow, INodeCredentials, INodeParameters, INodeTypeNameVersion, INodeTypes, } from 'n8n-workflow'; -import { Workflow } from 'n8n-workflow'; const TEMP_NODE_NAME = 'Temp-Node'; const TEMP_WORKFLOW_NAME = 'Temp-Workflow'; diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index 47258f615f..7c508e59fa 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -19,20 +19,23 @@ import type { IPinData, IRun, IRunData, - IRunExecutionData, ISourceData, ITaskData, ITaskDataConnections, ITaskDataConnectionsSource, IWaitingForExecution, IWaitingForExecutionSource, - IWorkflowExecuteAdditionalData, NodeApiError, NodeOperationError, Workflow, +} from 'n8n-workflow'; +import { + LoggerProxy as Logger, + WorkflowOperationError, + IRunExecutionData, + IWorkflowExecuteAdditionalData, WorkflowExecuteMode, } from 'n8n-workflow'; -import { LoggerProxy as Logger, WorkflowOperationError } from 'n8n-workflow'; import get from 'lodash/get'; import * as NodeExecuteFunctions from './NodeExecuteFunctions'; diff --git a/packages/core/src/decorators/LogCatch.decorator.ts b/packages/core/src/decorators/LogCatch.decorator.ts new file mode 100644 index 0000000000..f43bc71d61 --- /dev/null +++ b/packages/core/src/decorators/LogCatch.decorator.ts @@ -0,0 +1,30 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +/* eslint-disable @typescript-eslint/no-unsafe-call */ +/* eslint-disable @typescript-eslint/no-unsafe-member-access */ +/* eslint-disable @typescript-eslint/no-unsafe-assignment */ + +export const LogCatch = (logFn: (error: unknown) => void) => { + return (target: unknown, propertyKey: string, descriptor: PropertyDescriptor) => { + const originalMethod = descriptor.value; + + descriptor.value = function (...args: unknown[]) { + try { + const result: unknown = originalMethod.apply(this, args); + + if (result && result instanceof Promise) { + return result.catch((error: unknown) => { + logFn(error); + throw error; + }); + } + + return result; + } catch (error) { + logFn(error); + throw error; + } + }; + + return descriptor; + }; +}; diff --git a/packages/core/test/helpers/index.ts b/packages/core/test/helpers/index.ts index e6550f1b4e..8ffe9159e1 100644 --- a/packages/core/test/helpers/index.ts +++ b/packages/core/test/helpers/index.ts @@ -13,7 +13,6 @@ import type { INode, INodeCredentialsDetails, INodeType, - INodeTypeData, INodeTypes, IRun, ITaskData, @@ -24,7 +23,7 @@ import type { WorkflowTestData, } from 'n8n-workflow'; -import { ICredentialsHelper, NodeHelpers, WorkflowHooks } from 'n8n-workflow'; +import { ICredentialsHelper, NodeHelpers, WorkflowHooks, INodeTypeData } from 'n8n-workflow'; import { Credentials } from '@/Credentials'; import { predefinedNodesTypes } from './constants'; diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json index d69a889767..23a5a4a194 100644 --- a/packages/core/tsconfig.json +++ b/packages/core/tsconfig.json @@ -7,6 +7,8 @@ "paths": { "@/*": ["./*"] }, + "emitDecoratorMetadata": true, + "experimentalDecorators": true, // TODO: remove all options below this line "useUnknownInCatchVariables": false },