From 34f77c059b5a1176e5299c225fc3979125fba6d0 Mon Sep 17 00:00:00 2001 From: Emilia <100027345+sovietspaceship@users.noreply.github.com> Date: Mon, 28 Sep 2026 07:20:45 +0000 Subject: [PATCH] fix(core): Keep one failed binary-data dir from aborting the delete batch (#39194) Co-authored-by: Claude Opus 5 --- .../__tests__/blob.manager.test.ts | 33 +++++++++++++++++++ packages/core/src/binary-data/blob.manager.ts | 15 ++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/core/src/binary-data/__tests__/blob.manager.test.ts b/packages/core/src/binary-data/__tests__/blob.manager.test.ts index c4b11be3de9..1c3a3f7ca20 100644 --- a/packages/core/src/binary-data/__tests__/blob.manager.test.ts +++ b/packages/core/src/binary-data/__tests__/blob.manager.test.ts @@ -184,4 +184,37 @@ describe('deletion', () => { expect(byteStore.delete).not.toHaveBeenCalled(); }); + + it('deleteMany deletes the rest of the batch when one prefix fails, and warns', async () => { + const prefixStore = mock(); + const failure = new Error('ENOTEMPTY: directory not empty'); + prefixStore.deletePrefix = vi + .fn() + .mockRejectedValueOnce(failure) + .mockResolvedValueOnce(undefined); + const prefixManager = new BinaryDataBlobManager(prefixStore, errorReporter); + const other = { type: 'execution', workflowId, executionId: '1000' } as const; + + await expect(prefixManager.deleteMany([location, other])).resolves.toBeUndefined(); + + expect(prefixStore.deletePrefix).toHaveBeenCalledTimes(2); + expect(prefixStore.deletePrefix).toHaveBeenLastCalledWith( + `workflows/${workflowId}/executions/1000/binary_data`, + ); + expect(errorReporter.warn).toHaveBeenCalledTimes(1); + expect(errorReporter.warn).toHaveBeenCalledWith(failure, { + extra: { prefix: `workflows/${workflowId}/executions/${executionId}/binary_data` }, + }); + }); + + it('deleteMany resolves and warns for each prefix when every one fails', async () => { + const prefixStore = mock(); + prefixStore.deletePrefix = vi.fn().mockRejectedValue(new Error('ENOTEMPTY')); + const prefixManager = new BinaryDataBlobManager(prefixStore, errorReporter); + const other = { type: 'execution', workflowId, executionId: '1000' } as const; + + await expect(prefixManager.deleteMany([location, other])).resolves.toBeUndefined(); + + expect(errorReporter.warn).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/core/src/binary-data/blob.manager.ts b/packages/core/src/binary-data/blob.manager.ts index 28726f960a9..e69bd9f66a5 100644 --- a/packages/core/src/binary-data/blob.manager.ts +++ b/packages/core/src/binary-data/blob.manager.ts @@ -195,7 +195,20 @@ export class BinaryDataBlobManager implements BinaryData.Manager { const prefixes = [ ...new Set(locations.map((location) => `${this.toRelativePath(location)}/binary_data`)), ]; - await Promise.all(prefixes.map(async (prefix) => await deletePrefix(prefix))); + const results = await Promise.allSettled( + prefixes.map(async (prefix) => await deletePrefix(prefix)), + ); + + // Report an unremovable dir instead of throwing: callers delete in batches, and + // a throw would abandon every later batch. + results.forEach((result, index) => { + if (result.status === 'rejected') { + const reason: unknown = result.reason; + this.errorReporter.warn(reason instanceof Error ? reason : new Error(String(reason)), { + extra: { prefix: prefixes[index] }, + }); + } + }); } private metadataKey(fileId: string) {