fix(core): Keep one failed binary-data dir from aborting the delete batch (#39194)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Emilia 2026-09-28 07:20:45 +00:00 committed by GitHub
parent 7bebbe4033
commit 34f77c059b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 47 additions and 1 deletions

View File

@ -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<ByteStore>();
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<ByteStore>();
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);
});
});

View File

@ -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) {