From 5e632cad7cfdf7efcab1742e1e8df2ed873af709 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Tue, 19 Dec 2023 08:29:04 +0000 Subject: [PATCH 1/5] fix: Prevent binary data restoration from preventing execution from finishing --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 1f6bcc627b4e9..6409b34a0698f 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -410,7 +410,14 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { workflowId: this.workflowData.id, }); - await restoreBinaryDataId(fullRunData, this.executionId, this.mode); + try { + await restoreBinaryDataId(fullRunData, this.executionId, this.mode); + } catch (e) { + logger.debug('Failed to restore binary data ID', { + executionId: this.executionId, + mode: this.mode, + }); + } const isManualMode = [this.mode, parentProcessMode].includes('manual'); From a7898e63ad39e7d0c2a37666788c28b40081d84c Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Tue, 19 Dec 2023 13:11:43 +0000 Subject: [PATCH 2/5] refactor: Change logging level to error --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 6409b34a0698f..631a22adb47ff 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -413,7 +413,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { try { await restoreBinaryDataId(fullRunData, this.executionId, this.mode); } catch (e) { - logger.debug('Failed to restore binary data ID', { + logger.error('Failed to restore binary data ID', { executionId: this.executionId, mode: this.mode, }); From 944f6c68d170bcd5201e1ec2e351e49a1c0bee06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Dec 2023 13:13:11 +0100 Subject: [PATCH 3/5] refactor(core): Suggestions for #8082 --- .../cli/src/WorkflowExecuteAdditionalData.ts | 9 +--- .../restoreBinaryDataId.ts | 41 ++++++++++++------- .../restoreBinaryDataId.test.ts | 22 ++++++++++ 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 631a22adb47ff..1f6bcc627b4e9 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -410,14 +410,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { workflowId: this.workflowData.id, }); - try { - await restoreBinaryDataId(fullRunData, this.executionId, this.mode); - } catch (e) { - logger.error('Failed to restore binary data ID', { - executionId: this.executionId, - mode: this.mode, - }); - } + await restoreBinaryDataId(fullRunData, this.executionId, this.mode); const isManualMode = [this.mode, parentProcessMode].includes('manual'); diff --git a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts index 15c03e1b4df63..a973f50fea97d 100644 --- a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts +++ b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts @@ -3,6 +3,7 @@ import { BinaryDataService } from 'n8n-core'; import type { IRun, WorkflowExecuteMode } from 'n8n-workflow'; import type { BinaryData } from 'n8n-core'; import config from '@/config'; +import { Logger } from '@/Logger'; /** * Whenever the execution ID is not available to the binary data service at the @@ -32,28 +33,40 @@ export async function restoreBinaryDataId( return; } - const { runData } = run.data.resultData; + try { + const { runData } = run.data.resultData; - const promises = Object.keys(runData).map(async (nodeName) => { - const binaryDataId = runData[nodeName]?.[0]?.data?.main?.[0]?.[0]?.binary?.data?.id; + const promises = Object.keys(runData).map(async (nodeName) => { + const binaryDataId = runData[nodeName]?.[0]?.data?.main?.[0]?.[0]?.binary?.data?.id; - if (!binaryDataId) return; + if (!binaryDataId) return; - const [mode, fileId] = binaryDataId.split(':') as [BinaryData.StoredMode, string]; + const [mode, fileId] = binaryDataId.split(':') as [BinaryData.StoredMode, string]; - const isMissingExecutionId = fileId.includes('/temp/'); + const isMissingExecutionId = fileId.includes('/temp/'); - if (!isMissingExecutionId) return; + if (!isMissingExecutionId) return; - const correctFileId = fileId.replace('temp', executionId); + const correctFileId = fileId.replace('temp', executionId); - await Container.get(BinaryDataService).rename(fileId, correctFileId); + await Container.get(BinaryDataService).rename(fileId, correctFileId); - const correctBinaryDataId = `${mode}:${correctFileId}`; + const correctBinaryDataId = `${mode}:${correctFileId}`; - // @ts-expect-error Validated at the top - run.data.resultData.runData[nodeName][0].data.main[0][0].binary.data.id = correctBinaryDataId; - }); + // @ts-expect-error Validated at the top + run.data.resultData.runData[nodeName][0].data.main[0][0].binary.data.id = correctBinaryDataId; + }); - await Promise.all(promises); + await Promise.all(promises); + } catch (e) { + const error = e instanceof Error ? e : new Error(`${e}`); + const logger = Container.get(Logger); + + if (error.message.includes('ENOENT')) { + logger.warn('Failed to restore binary data ID - No such file or dir', { executionId, error }); + return; + } + + logger.warn('Failed to restore binary data ID - Unknown error', { executionId, error }); + } } diff --git a/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts b/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts index 05d828100cbac..3fcdb79c72c7e 100644 --- a/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts +++ b/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts @@ -142,6 +142,28 @@ for (const mode of ['filesystem-v2', 's3'] as const) { expect(binaryDataService.rename).not.toHaveBeenCalled(); }); + + it('should ignore error thrown on renaming', async () => { + const workflowId = '6HYhhKmJch2cYxGj'; + const executionId = 'temp'; + const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6'; + + const incorrectFileId = `workflows/${workflowId}/executions/temp/binary_data/${binaryDataFileUuid}`; + + const run = toIRun({ + binary: { + data: { id: `s3:${incorrectFileId}` }, + }, + }); + + binaryDataService.rename.mockRejectedValueOnce(new Error('ENOENT')); + + const promise = restoreBinaryDataId(run, executionId, 'webhook'); + + await expect(promise).resolves.not.toThrow(); + + expect(binaryDataService.rename).toHaveBeenCalled(); + }); }); } From 66b8ef2fdf04d408e6502c3517e68331bed6e87b Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Wed, 20 Dec 2023 09:29:32 +0000 Subject: [PATCH 4/5] Update packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iván Ovejero --- .../cli/src/executionLifecycleHooks/restoreBinaryDataId.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts index a973f50fea97d..6cb8b796eeed7 100644 --- a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts +++ b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts @@ -63,10 +63,10 @@ export async function restoreBinaryDataId( const logger = Container.get(Logger); if (error.message.includes('ENOENT')) { - logger.warn('Failed to restore binary data ID - No such file or dir', { executionId, error }); + logger.error('Failed to restore binary data ID - No such file or dir', { executionId, error }); return; } - logger.warn('Failed to restore binary data ID - Unknown error', { executionId, error }); + logger.error('Failed to restore binary data ID - Unknown error', { executionId, error }); } } From ddc2051c6c1842506b8acc3b462021ea1efd1300 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Wed, 20 Dec 2023 16:43:35 +0000 Subject: [PATCH 5/5] fix: Lint issue --- .../cli/src/executionLifecycleHooks/restoreBinaryDataId.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts index 6cb8b796eeed7..84780a785e410 100644 --- a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts +++ b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts @@ -63,7 +63,10 @@ export async function restoreBinaryDataId( const logger = Container.get(Logger); if (error.message.includes('ENOENT')) { - logger.error('Failed to restore binary data ID - No such file or dir', { executionId, error }); + logger.warn('Failed to restore binary data ID - No such file or dir', { + executionId, + error, + }); return; }