Skip to content

Commit 52b5c2f

Browse files
committed
fix(files): only remap the deadline abort itself, and stop overclaiming rollback
Two follow-ups on the budget deadline, both reported by Cursor Bugbot: `deadline.aborted` stays true for the rest of the request once the timer fires, so it cannot decide whether *this* error was the abort. An `ArchiveError` or storage failure thrown mid-entry after the timer fired was being relabelled as a timeout and returned as a 413, hiding the real cause. The catch now matches the thrown value against `deadline.reason` — `throwIfAborted()` throws exactly that object, so the check is identity-exact and cannot capture an unrelated failure. The message also claimed a rollback that has not necessarily happened: the budget covers the archive download too, so it can fire before the first write, when there is nothing to roll back. It now says the unzip was cancelled and claims nothing about what was written. Including the download in the budget is deliberate — the lease it has to fit inside starts earlier still — so the TSDoc says that rather than "the extraction itself".
1 parent ef57474 commit 52b5c2f

2 files changed

Lines changed: 43 additions & 13 deletions

File tree

apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,13 +286,18 @@ describe('extractWorkspaceFile', () => {
286286
expect(mocks.notify).toHaveBeenCalledWith('workspace-1')
287287
})
288288

289+
/** Fires the deadline the way `AbortSignal.timeout` does: `reason` is what gets thrown. */
290+
function expireDeadline(signal: AbortSignal): unknown {
291+
const reason = new DOMException('The operation was aborted due to timeout', 'TimeoutError')
292+
Object.defineProperty(signal, 'aborted', { value: true })
293+
Object.defineProperty(signal, 'reason', { value: reason })
294+
return reason
295+
}
296+
289297
it('reports a budget overrun as a caller-fixable error, not the raw abort', async () => {
290298
mocks.decompress.mockImplementationOnce(async (_content, options) => {
291299
await options.prepareRootFolder()
292-
// Stand in for the deadline firing mid-write: the extractor aborts, rolls back, and
293-
// rethrows the DOMException, which must not reach the caller as an opaque 500.
294-
Object.defineProperty(options.signal, 'aborted', { value: true })
295-
throw Object.assign(new Error('The operation was aborted'), { name: 'TimeoutError' })
300+
throw expireDeadline(options.signal)
296301
})
297302

298303
await expect(
@@ -302,12 +307,35 @@ describe('extractWorkspaceFile', () => {
302307
})
303308
).rejects.toMatchObject({
304309
code: 'payload_too_large',
305-
message: expect.stringContaining('took too long and was rolled back'),
310+
message: expect.stringContaining('took too long and was cancelled'),
306311
})
307312

308313
expect(mocks.archiveFolderIfEmpty).toHaveBeenCalledOnce()
309314
})
310315

316+
it('keeps the real cause when a failure races the deadline', async () => {
317+
// The signal stays aborted for the rest of the request, so an unrelated mid-entry
318+
// failure after the timer fires must not be relabelled as a timeout.
319+
mocks.decompress.mockImplementationOnce(async (_content, options) => {
320+
await options.prepareRootFolder()
321+
expireDeadline(options.signal)
322+
throw Object.assign(new Error('Archive entry "a.txt" could not be decompressed'), {
323+
name: 'ArchiveError',
324+
reason: 'invalid',
325+
})
326+
})
327+
328+
await expect(
329+
extractWorkspaceFile.execute({
330+
principal,
331+
input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' },
332+
})
333+
).rejects.toMatchObject({
334+
name: 'ArchiveError',
335+
message: expect.stringContaining('could not be decompressed'),
336+
})
337+
})
338+
311339
it('leaves a destination folder that gained collaborators content during rollback', async () => {
312340
mocks.decompress.mockImplementationOnce(async (_content, options) => {
313341
await options.prepareRootFolder()

apps/sim/lib/workspace-files/application/extract-workspace-file.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,12 @@ import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folde
2525

2626
const logger = createLogger('ExtractWorkspaceFile')
2727
/**
28-
* Wall-clock budget for the extraction itself, deliberately shorter than the route's
29-
* `maxDuration` so the work stops on our terms — with the all-or-nothing rollback still
30-
* able to run — rather than the platform killing the process mid-write and stranding a
31-
* partial tree. Self-hosted deployments do not enforce `maxDuration` at all, so this is
32-
* the only thing bounding the write loop there.
28+
* Wall-clock budget for the whole operation — archive download included, because the lease
29+
* this must fit inside starts earlier still. Deliberately shorter than the route's
30+
* `maxDuration` so the work stops on our terms, with the all-or-nothing rollback still able
31+
* to run, rather than the platform killing the process mid-write and stranding a partial
32+
* tree. Self-hosted deployments do not enforce `maxDuration` at all, so this is the only
33+
* thing bounding the write loop there.
3334
*/
3435
const EXTRACTION_BUDGET_MS = 180 * 1000
3536
/**
@@ -198,15 +199,16 @@ async function extractWorkspaceFileContents({
198199
}
199200
await notifyWorkspaceFilesChanged(context.workspaceId)
200201
}
201-
if (deadline.aborted && !(error instanceof OrchestrationError)) {
202-
logger.warn('Archive extraction exceeded its budget and was rolled back', {
202+
if (deadline.aborted && error === deadline.reason) {
203+
logger.warn('Archive extraction exceeded its budget', {
203204
workspaceId: context.workspaceId,
204205
fileId: file.id,
205206
budgetMs: EXTRACTION_BUDGET_MS,
207+
wroteAnything: Boolean(rootFolder),
206208
})
207209
throw new OrchestrationError(
208210
'payload_too_large',
209-
`Unzipping "${file.name}" took too long and was rolled back. Try a smaller archive.`
211+
`Unzipping "${file.name}" took too long and was cancelled. Try a smaller archive.`
210212
)
211213
}
212214
throw error

0 commit comments

Comments
 (0)