mirror of
https://github.com/n8n-io/n8n.git
synced 2024-11-15 09:04:07 -08:00
5ffff1bb22
In the case of a filesystem failure to rename the binary files as part of the execution's cleanup process, the execution would fail to be saved and would never finish. This catch prevents it. ## Summary Whenever an execution is wrapping u to save the data, if it uses binary data n8n will try to find possibly misallocated files and place them in the right folder. If this process fails, the execution fails to finish. Given the execution has already finished at this point, and we cannot handle the binary data errors more gracefully, all we can do at this point is log the message as it's a filesystem issue. The rest of the execution saving process should remain as normal. ## Related tickets and issues https://linear.app/n8n/issue/HELP-430 ## 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. --------- Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
193 lines
4.7 KiB
TypeScript
193 lines
4.7 KiB
TypeScript
import { restoreBinaryDataId } from '@/executionLifecycleHooks/restoreBinaryDataId';
|
|
import { BinaryDataService } from 'n8n-core';
|
|
import { mockInstance } from '../../shared/mocking';
|
|
|
|
import type { IRun } from 'n8n-workflow';
|
|
import config from '@/config';
|
|
|
|
function toIRun(item?: object) {
|
|
return {
|
|
data: {
|
|
resultData: {
|
|
runData: {
|
|
myNode: [
|
|
{
|
|
data: {
|
|
main: [[item]],
|
|
},
|
|
},
|
|
],
|
|
},
|
|
},
|
|
},
|
|
} as unknown as IRun;
|
|
}
|
|
|
|
function getDataId(run: IRun, kind: 'binary' | 'json') {
|
|
// @ts-ignore
|
|
return run.data.resultData.runData.myNode[0].data.main[0][0][kind].data.id;
|
|
}
|
|
|
|
const binaryDataService = mockInstance(BinaryDataService);
|
|
|
|
for (const mode of ['filesystem-v2', 's3'] as const) {
|
|
describe(`on ${mode} mode`, () => {
|
|
beforeAll(() => {
|
|
config.set('binaryDataManager.mode', mode);
|
|
});
|
|
|
|
afterEach(() => {
|
|
jest.clearAllMocks();
|
|
});
|
|
|
|
it('should restore if binary data ID is missing execution ID', 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}` },
|
|
},
|
|
});
|
|
|
|
await restoreBinaryDataId(run, executionId, 'webhook');
|
|
|
|
const correctFileId = incorrectFileId.replace('temp', executionId);
|
|
const correctBinaryDataId = `s3:${correctFileId}`;
|
|
|
|
expect(binaryDataService.rename).toHaveBeenCalledWith(incorrectFileId, correctFileId);
|
|
expect(getDataId(run, 'binary')).toBe(correctBinaryDataId);
|
|
});
|
|
|
|
it('should do nothing if binary data ID is not missing execution ID', async () => {
|
|
const workflowId = '6HYhhKmJch2cYxGj';
|
|
const executionId = '999';
|
|
const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6';
|
|
|
|
const fileId = `workflows/${workflowId}/executions/${executionId}/binary_data/${binaryDataFileUuid}`;
|
|
|
|
const binaryDataId = `s3:${fileId}`;
|
|
|
|
const run = toIRun({
|
|
binary: {
|
|
data: {
|
|
id: binaryDataId,
|
|
},
|
|
},
|
|
});
|
|
|
|
await restoreBinaryDataId(run, executionId, 'webhook');
|
|
|
|
expect(binaryDataService.rename).not.toHaveBeenCalled();
|
|
expect(getDataId(run, 'binary')).toBe(binaryDataId);
|
|
});
|
|
|
|
it('should do nothing if no binary data ID', async () => {
|
|
const executionId = '999';
|
|
const dataId = '123';
|
|
|
|
const run = toIRun({
|
|
json: {
|
|
data: { id: dataId },
|
|
},
|
|
});
|
|
|
|
await restoreBinaryDataId(run, executionId, 'webhook');
|
|
|
|
expect(binaryDataService.rename).not.toHaveBeenCalled();
|
|
expect(getDataId(run, 'json')).toBe(dataId);
|
|
});
|
|
|
|
it('should do nothing on itemless case', async () => {
|
|
const executionId = '999';
|
|
|
|
const promise = restoreBinaryDataId(toIRun(), executionId, 'webhook');
|
|
|
|
await expect(promise).resolves.not.toThrow();
|
|
|
|
expect(binaryDataService.rename).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('should do nothing if data is undefined', async () => {
|
|
const executionId = '999';
|
|
|
|
const run = toIRun({
|
|
json: {
|
|
data: undefined,
|
|
},
|
|
});
|
|
|
|
const promise = restoreBinaryDataId(run, executionId, 'webhook');
|
|
|
|
await expect(promise).resolves.not.toThrow();
|
|
|
|
expect(binaryDataService.rename).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('should do nothing if workflow execution mode is not `webhook`', async () => {
|
|
const executionId = '999';
|
|
|
|
const run = toIRun({
|
|
json: {
|
|
data: undefined,
|
|
},
|
|
});
|
|
|
|
const promise = restoreBinaryDataId(run, executionId, 'internal');
|
|
|
|
await expect(promise).resolves.not.toThrow();
|
|
|
|
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();
|
|
});
|
|
});
|
|
}
|
|
|
|
describe('on default mode', () => {
|
|
afterEach(() => {
|
|
config.load(config.default);
|
|
});
|
|
|
|
it('should do nothing', async () => {
|
|
config.set('binaryDataManager.mode', 'default');
|
|
|
|
const executionId = '999';
|
|
|
|
const run = toIRun({
|
|
json: {
|
|
data: undefined,
|
|
},
|
|
});
|
|
|
|
const promise = restoreBinaryDataId(run, executionId, 'internal');
|
|
|
|
await expect(promise).resolves.not.toThrow();
|
|
|
|
expect(binaryDataService.rename).not.toHaveBeenCalled();
|
|
});
|
|
});
|