Skip to content

Commit 339b648

Browse files
committed
fix(files): attribute a size rejection to the cap that actually bound it
Every PayloadSizeLimitError was reported as the entry exceeding its per-document render allowance, quoting the 50 MiB headroom. That is wrong when the shared budget was the smaller cap, and wrong for extensions that carry no headroom at all — a video rejected because the budget ran out was told it exceeded a per-document render limit. The entry is now blamed only when its own allowance was the smaller of the two caps, and the message quotes the allowance that applied rather than the constant. The mapper also returns its outcome instead of mutating closure state, which is what the aggregate and per-entry branches now read.
1 parent 8a02ca6 commit 339b648

2 files changed

Lines changed: 55 additions & 24 deletions

File tree

apps/sim/app/api/workspaces/[id]/files/download/route.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,21 @@ describe('workspace files download route', () => {
137137
expect(body.error).not.toContain('Selected files total')
138138
})
139139

140+
it('blames the shared budget, not the entry, when the entry had no smaller cap', async () => {
141+
// .mp4 has no render headroom, so its cap is whatever is left of the budget.
142+
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'clip.mp4', 'folder-1')])
143+
mockFetchServableWorkspaceFileBuffer.mockRejectedValue(
144+
new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
145+
)
146+
147+
const response = await GET(requestFor('fileIds=f1'), context)
148+
149+
expect(response.status).toBe(400)
150+
const body = await response.json()
151+
expect(body.error).toContain('Selected files total')
152+
expect(body.error).not.toContain('clip.mp4')
153+
})
154+
140155
it('lets an uploaded office file larger than the render headroom through', async () => {
141156
const big = { ...workspaceFile('f1', 'deck.pptx', 'folder-1'), size: 80 * 1024 * 1024 }
142157
mockListWorkspaceFiles.mockResolvedValue([big])

apps/sim/app/api/workspaces/[id]/files/download/route.ts

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -125,68 +125,84 @@ export const GET = withRouteHandler(
125125
// abort flag stops reads that have not started yet.
126126
const controller = new AbortController()
127127
let renderedBytes = 0
128-
let overLimit = false
129-
let overLimitFileName: string | null = null
128+
129+
interface DownloadOutcome {
130+
buffer: Buffer | null
131+
pendingName: string | null
132+
/** Set only when an entry's own allowance bound it, never the shared budget. */
133+
overLimitEntry: { name: string; allowance: number } | null
134+
overLimit: boolean
135+
error: unknown
136+
}
137+
const skipped: DownloadOutcome = {
138+
buffer: null,
139+
pendingName: null,
140+
overLimitEntry: null,
141+
overLimit: false,
142+
error: null,
143+
}
130144

131145
const downloads = await mapWithConcurrency(
132146
filesToZip,
133147
ZIP_MATERIALIZE_CONCURRENCY,
134-
async (file) => {
135-
if (controller.signal.aborted) return { buffer: null, pendingName: null, error: null }
148+
async (file): Promise<DownloadOutcome> => {
149+
if (controller.signal.aborted) return skipped
136150
const remaining = Math.max(0, MAX_ZIP_DOWNLOAD_BYTES - renderedBytes)
151+
const allowance = isRenderableDocumentName(file.name)
152+
? Math.max(file.size, RENDERED_DOCUMENT_HEADROOM_BYTES)
153+
: remaining
137154
try {
138-
const allowance = isRenderableDocumentName(file.name)
139-
? Math.max(file.size, RENDERED_DOCUMENT_HEADROOM_BYTES)
140-
: remaining
141155
const { buffer } = await fetchServableWorkspaceFileBuffer(file, {
142156
maxBytes: Math.min(remaining, allowance),
143157
signal: controller.signal,
144158
})
145159
renderedBytes += buffer.length
146-
if (renderedBytes > MAX_ZIP_DOWNLOAD_BYTES) {
147-
overLimit = true
148-
controller.abort()
149-
}
150-
return { buffer, pendingName: null, error: null }
160+
const overLimit = renderedBytes > MAX_ZIP_DOWNLOAD_BYTES
161+
if (overLimit) controller.abort()
162+
return { ...skipped, buffer, overLimit }
151163
} catch (error) {
152164
// Recorded even when another worker already aborted: a size rejection
153165
// describes this file, so losing it to someone else's cancellation would
154166
// downgrade an actionable 400 into an opaque 500.
155167
if (error instanceof PayloadSizeLimitError) {
156-
overLimit = true
157-
overLimitFileName ??= file.name
158168
controller.abort()
159-
return { buffer: null, pendingName: null, error: null }
169+
// Attributed to this entry only when its own allowance was the smaller
170+
// of the two caps; otherwise the shared budget is what ran out.
171+
return {
172+
...skipped,
173+
overLimit: true,
174+
overLimitEntry: allowance < remaining ? { name: file.name, allowance } : null,
175+
}
160176
}
161177
// Any other error from an already-aborted read is a consequence of the
162178
// cancellation, not a cause. Checked before this worker aborts anything so
163179
// the worker that actually failed still records its own error.
164-
if (controller.signal.aborted) {
165-
return { buffer: null, pendingName: null, error: null }
166-
}
180+
if (controller.signal.aborted) return skipped
167181
// A pending artifact is worth reporting in full, so keep resolving the
168182
// rest of the selection; anything else dooms the request.
169183
const pending = isDocNotReadyError(error)
170184
if (!pending) controller.abort()
171-
return { buffer: null, pendingName: pending ? file.name : null, error }
185+
return { ...skipped, pendingName: pending ? file.name : null, error }
172186
}
173187
}
174188
)
175189

176190
// Size first: the request cannot succeed at any size-adjacent retry, and a
177191
// descriptive 400 beats an opaque 500 raised by whatever the abort cancelled.
178-
if (overLimitFileName) {
179-
// Naming the entry that blew its own allowance: an aggregate message here
180-
// would tell the user to select fewer files when the selection was fine.
192+
const overLimitEntry = downloads.find((result) => result.overLimitEntry)?.overLimitEntry
193+
if (overLimitEntry) {
194+
// Naming the entry that blew its own allowance, and quoting the allowance that
195+
// actually applied: an aggregate message here would tell the user to select
196+
// fewer files when the selection was fine.
181197
return NextResponse.json(
182198
{
183-
error: `"${overLimitFileName}" is too large to include in a zip. A single document may render up to ${formatFileSize(RENDERED_DOCUMENT_HEADROOM_BYTES)}; download it on its own instead.`,
199+
error: `"${overLimitEntry.name}" is too large to include in a zip. Entries are capped at ${formatFileSize(overLimitEntry.allowance)}; download it on its own instead.`,
184200
},
185201
{ status: 400 }
186202
)
187203
}
188204

189-
if (overLimit || renderedBytes > MAX_ZIP_DOWNLOAD_BYTES) {
205+
if (downloads.some((result) => result.overLimit)) {
190206
return overLimitResponse(renderedBytes, ' once documents are rendered')
191207
}
192208

0 commit comments

Comments
 (0)