Skip to content

Commit 80a4e0c

Browse files
committed
perf(files): stream workspace archives instead of buffering them
The zip route materialized every selected file before writing a byte, so peak memory tracked the size of the selection. Ordinary files are now appended as lazy streams: each opens its storage read only when the archiver reaches it, so one entry is resident at a time rather than the whole archive. Generated documents still resolve to buffers first. They are the only entries whose bytes are needed to decide anything, and every status this route returns comes from them — once the first byte is written the status code is committed, so those decisions have to happen before the archive starts. Uses archiver, whose entry queue processes appends sequentially, with lazystream deferring each storage read; handing the archiver one open stream per entry would hold more connections than the storage client pools. The Node-to-web bridge that input-validation.server.ts already hardened moves to a shared util: Readable.toWeb throws ERR_INVALID_STATE when a consumer cancels mid-transfer, which is exactly what a cancelled download does. Trade: a storage read that fails mid-archive now truncates the response instead of returning 500, since the status is already sent. Matches how the table export route behaves.
1 parent 95942d2 commit 80a4e0c

6 files changed

Lines changed: 320 additions & 246 deletions

File tree

Lines changed: 100 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { Readable } from 'stream'
45
import { createMockRequest } from '@sim/testing'
5-
import { sleep } from '@sim/utils/helpers'
66
import JSZip from 'jszip'
77
import { beforeEach, describe, expect, it, vi } from 'vitest'
88

@@ -12,12 +12,14 @@ const {
1212
mockListWorkspaceFiles,
1313
mockListWorkspaceFileFolders,
1414
mockFetchServableWorkspaceFileBuffer,
15+
mockDownloadFileStream,
1516
} = vi.hoisted(() => ({
1617
mockGetSession: vi.fn(),
1718
mockVerifyWorkspaceMembership: vi.fn(),
1819
mockListWorkspaceFiles: vi.fn(),
1920
mockListWorkspaceFileFolders: vi.fn(),
2021
mockFetchServableWorkspaceFileBuffer: vi.fn(),
22+
mockDownloadFileStream: vi.fn(),
2123
}))
2224

2325
vi.mock('@/lib/auth', () => ({
@@ -37,6 +39,10 @@ vi.mock('@/lib/uploads/contexts/workspace', () => ({
3739
fetchServableWorkspaceFileBuffer: mockFetchServableWorkspaceFileBuffer,
3840
}))
3941

42+
vi.mock('@/lib/uploads/core/storage-service', () => ({
43+
downloadFileStream: mockDownloadFileStream,
44+
}))
45+
4046
vi.mock('@sim/audit', () => ({
4147
recordAudit: vi.fn(),
4248
AuditAction: { FILE_DOWNLOADED: 'file.downloaded' },
@@ -51,8 +57,9 @@ import { GET } from '@/app/api/workspaces/[id]/files/download/route'
5157

5258
const WORKSPACE_ID = 'ws-1'
5359
const context = { params: Promise.resolve({ id: WORKSPACE_ID }) }
60+
const MB = 1024 * 1024
5461

55-
function workspaceFile(id: string, name: string, folderId: string | null) {
62+
function workspaceFile(id: string, name: string, folderId: string | null = 'folder-1') {
5663
return {
5764
id,
5865
name,
@@ -73,6 +80,10 @@ function requestFor(query: string) {
7380
)
7481
}
7582

83+
async function zipFrom(response: Response) {
84+
return JSZip.loadAsync(Buffer.from(await response.arrayBuffer()))
85+
}
86+
7687
describe('workspace files download route', () => {
7788
beforeEach(() => {
7889
vi.clearAllMocks()
@@ -81,12 +92,13 @@ describe('workspace files download route', () => {
8192
mockListWorkspaceFileFolders.mockResolvedValue([
8293
{ id: 'folder-1', name: 'Reports', parentId: null },
8394
])
95+
mockDownloadFileStream.mockImplementation(async () => Readable.from([Buffer.from('plain')]))
8496
})
8597

8698
it('zips the rendered bytes for a generated doc, not its stored source', async () => {
87-
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'overview.docx', 'folder-1')])
99+
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'overview.docx')])
88100
// A real .docx is a ZIP; the stored source would be plain JS text.
89-
const rendered = Buffer.from('PKrendered-docx')
101+
const rendered = Buffer.from('PKrendered-docx')
90102
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
91103
buffer: rendered,
92104
contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
@@ -95,162 +107,146 @@ describe('workspace files download route', () => {
95107
const response = await GET(requestFor('fileIds=f1'), context)
96108

97109
expect(response.status).toBe(200)
98-
expect(mockFetchServableWorkspaceFileBuffer).toHaveBeenCalledTimes(1)
99-
100-
const zip = await JSZip.loadAsync(Buffer.from(await response.arrayBuffer()))
101-
const entry = zip.file('Reports/overview.docx')
110+
const entry = (await zipFrom(response)).file('Reports/overview.docx')
102111
expect(entry).not.toBeNull()
103112
expect(Buffer.from(await entry!.async('uint8array'))).toEqual(rendered)
104113
})
105114

115+
it('streams ordinary files instead of materializing them', async () => {
116+
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'clip.mp4')])
117+
118+
const response = await GET(requestFor('fileIds=f1'), context)
119+
120+
expect(response.status).toBe(200)
121+
// Nothing has been read yet: the entry opens its storage read only once the
122+
// consumer pulls the archive, which is what keeps peak memory to one entry.
123+
expect(mockDownloadFileStream).not.toHaveBeenCalled()
124+
125+
const zip = await zipFrom(response)
126+
127+
expect(mockDownloadFileStream).toHaveBeenCalledTimes(1)
128+
// Never routed through the buffering document reader.
129+
expect(mockFetchServableWorkspaceFileBuffer).not.toHaveBeenCalled()
130+
131+
const entry = zip.file('Reports/clip.mp4')
132+
expect(entry).not.toBeNull()
133+
expect(await entry!.async('string')).toBe('plain')
134+
})
135+
136+
it('preserves nested folder paths across both entry kinds', async () => {
137+
mockListWorkspaceFileFolders.mockResolvedValue([
138+
{ id: 'folder-1', name: 'Reports', parentId: null },
139+
{ id: 'folder-2', name: 'visuals', parentId: 'folder-1' },
140+
])
141+
mockListWorkspaceFiles.mockResolvedValue([
142+
workspaceFile('f1', 'summary.docx', 'folder-1'),
143+
workspaceFile('f2', 'hero.png', 'folder-2'),
144+
])
145+
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
146+
buffer: Buffer.from('PKdoc'),
147+
contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
148+
})
149+
150+
const zip = await zipFrom(await GET(requestFor('fileIds=f1&fileIds=f2'), context))
151+
152+
expect(zip.file('Reports/summary.docx')).not.toBeNull()
153+
expect(zip.file('visuals/hero.png')).not.toBeNull()
154+
})
155+
106156
it('returns 409 naming the documents whose artifacts are still compiling', async () => {
107157
mockListWorkspaceFiles.mockResolvedValue([
108-
workspaceFile('f1', 'ready.md', 'folder-1'),
109-
workspaceFile('f2', 'pending.docx', 'folder-1'),
158+
workspaceFile('f1', 'ready.docx'),
159+
workspaceFile('f2', 'pending.docx'),
110160
])
111161
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
112162
if (file.name === 'pending.docx')
113163
throw new DocCompileUserError('Document is still being generated')
114-
return { buffer: Buffer.from('ok'), contentType: 'text/markdown' }
164+
return { buffer: Buffer.from('PKok'), contentType: 'application/octet-stream' }
115165
})
116166

117167
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
118168

119169
expect(response.status).toBe(409)
120170
const body = await response.json()
121171
expect(body.error).toContain('pending.docx')
122-
expect(body.error).not.toContain('ready.md')
172+
expect(body.error).not.toContain('ready.docx')
123173
})
124174

125-
it('rejects with 400, not 500, when a rendered document blows the byte budget', async () => {
126-
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'huge.docx', 'folder-1')])
175+
it('rejects with 400, not 500, when a document blows its own allowance', async () => {
176+
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'huge.docx')])
127177
mockFetchServableWorkspaceFileBuffer.mockRejectedValue(
128-
new PayloadSizeLimitError('servable file download exceeds limit')
178+
new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
129179
)
130180

131181
const response = await GET(requestFor('fileIds=f1'), context)
132182

133183
expect(response.status).toBe(400)
134-
// Names the offending entry rather than blaming the whole selection.
135184
const body = await response.json()
136185
expect(body.error).toContain('huge.docx')
137186
expect(body.error).not.toContain('Selected files total')
138187
})
139188

140-
it('blames the entry when its render ceiling exactly equals the remaining budget', async () => {
141-
// Declared at the full budget, so allowance === remaining and the caps tie.
142-
const doc = { ...workspaceFile('f1', 'report.docx', 'folder-1'), size: 250 * 1024 * 1024 }
143-
mockListWorkspaceFiles.mockResolvedValue([doc])
144-
mockFetchServableWorkspaceFileBuffer.mockRejectedValue(
145-
new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
146-
)
147-
148-
const response = await GET(requestFor('fileIds=f1'), context)
149-
150-
expect(response.status).toBe(400)
151-
// Downloading it on its own is still the way through, so name it.
152-
expect((await response.json()).error).toContain('report.docx')
153-
})
154-
155-
it('blames the shared budget, not the entry, when the entry had no smaller cap', async () => {
156-
// .mp4 has no render headroom, so its cap is whatever is left of the budget.
157-
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'clip.mp4', 'folder-1')])
158-
mockFetchServableWorkspaceFileBuffer.mockRejectedValue(
159-
new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
160-
)
189+
it('blames the shared budget once earlier documents have consumed it', async () => {
190+
mockListWorkspaceFiles.mockResolvedValue([
191+
workspaceFile('f1', 'first.docx'),
192+
workspaceFile('f2', 'second.docx'),
193+
])
194+
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
195+
// The first document eats the whole budget, so the second's cap is the remainder.
196+
if (file.name === 'first.docx') {
197+
return { buffer: Buffer.alloc(240 * MB), contentType: 'application/octet-stream' }
198+
}
199+
throw new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
200+
})
161201

162-
const response = await GET(requestFor('fileIds=f1'), context)
202+
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
163203

164204
expect(response.status).toBe(400)
165205
const body = await response.json()
166206
expect(body.error).toContain('Selected files total')
167-
expect(body.error).not.toContain('clip.mp4')
207+
expect(body.error).not.toContain('second.docx')
168208
})
169209

170210
it('lets an uploaded office file larger than the render headroom through', async () => {
171-
const big = { ...workspaceFile('f1', 'deck.pptx', 'folder-1'), size: 80 * 1024 * 1024 }
211+
const big = { ...workspaceFile('f1', 'deck.pptx'), size: 80 * MB }
172212
mockListWorkspaceFiles.mockResolvedValue([big])
173213
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
174-
buffer: Buffer.from('ok'),
214+
buffer: Buffer.from('PKdeck'),
175215
contentType: 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
176216
})
177217

178218
const response = await GET(requestFor('fileIds=f1'), context)
179219

180220
expect(response.status).toBe(200)
181221
// Capped at the declared size, not the smaller render headroom.
182-
expect(mockFetchServableWorkspaceFileBuffer.mock.calls[0][1].maxBytes).toBe(80 * 1024 * 1024)
183-
})
184-
185-
it('caps rendered documents per entry so concurrent reads cannot each claim the budget', async () => {
186-
mockListWorkspaceFiles.mockResolvedValue([
187-
workspaceFile('f1', 'report.docx', 'folder-1'),
188-
workspaceFile('f2', 'clip.mp4', 'folder-1'),
189-
])
190-
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
191-
buffer: Buffer.from('ok'),
192-
contentType: 'application/octet-stream',
193-
})
194-
195-
await GET(requestFor('fileIds=f1&fileIds=f2'), context)
196-
197-
const maxBytesFor = (name: string) =>
198-
mockFetchServableWorkspaceFileBuffer.mock.calls.find(
199-
(call: [{ name: string }, { maxBytes: number }]) => call[0].name === name
200-
)?.[1].maxBytes
201-
202-
// Only the source-backed document can render larger than it declares.
203-
expect(maxBytesFor('report.docx')).toBe(50 * 1024 * 1024)
204-
expect(maxBytesFor('clip.mp4')).toBe(250 * 1024 * 1024)
222+
expect(mockFetchServableWorkspaceFileBuffer.mock.calls[0][1].maxBytes).toBe(80 * MB)
205223
})
206224

207-
it('reports an oversized selection as 400 even when the abort cancels other reads', async () => {
208-
const files = Array.from({ length: 40 }, (_, index) =>
209-
workspaceFile(`f${index}`, `doc${index}.docx`, 'folder-1')
210-
)
211-
mockListWorkspaceFiles.mockResolvedValue(files)
212-
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
213-
if (file.name === 'doc0.docx')
214-
throw new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
215-
// Everything else fails the way a cancelled read would.
216-
throw new DOMException('The operation was aborted', 'AbortError')
217-
})
218-
219-
const response = await GET(
220-
requestFor(files.map((file) => `fileIds=${file.id}`).join('&')),
221-
context
222-
)
223-
224-
// Cancellation noise must not turn the size rejection into a generic 500.
225-
expect(response.status).toBe(400)
226-
})
227-
228-
it('keeps the size rejection when a hard failure aborted the read first', async () => {
225+
it('surfaces a storage failure as a 500 even when another document is pending', async () => {
229226
mockListWorkspaceFiles.mockResolvedValue([
230-
workspaceFile('f1', 'broken.txt', 'folder-1'),
231-
workspaceFile('f2', 'huge.docx', 'folder-1'),
227+
workspaceFile('f1', 'pending.docx'),
228+
workspaceFile('f2', 'broken.docx'),
232229
])
233230
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
234-
if (file.name === 'broken.txt') throw new Error('storage down')
235-
// Lands after the hard failure has already aborted the shared controller.
236-
await sleep(1)
237-
throw new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
231+
if (file.name === 'pending.docx')
232+
throw new DocCompileUserError('Document is still being generated')
233+
throw new Error('storage down')
238234
})
239235

240236
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
241237

242-
// "Select fewer files" is actionable; a generic 500 is not.
243-
expect(response.status).toBe(400)
238+
// A 409 would tell the client to retry something that can never succeed.
239+
expect(response.status).toBe(500)
244240
})
245241

246-
it('stops issuing reads once one hard-fails instead of draining the selection', async () => {
247-
const files = Array.from({ length: 60 }, (_, index) =>
248-
workspaceFile(`f${index}`, `doc${index}.txt`, 'folder-1')
242+
it('stops resolving documents once one hard-fails', async () => {
243+
const files = Array.from({ length: 20 }, (_, index) =>
244+
workspaceFile(`f${index}`, `doc${index}.docx`)
249245
)
250246
mockListWorkspaceFiles.mockResolvedValue(files)
251247
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
252-
if (file.name === 'doc0.txt') throw new Error('storage down')
253-
return { buffer: Buffer.from('ok'), contentType: 'text/plain' }
248+
if (file.name === 'doc0.docx') throw new Error('storage down')
249+
return { buffer: Buffer.from('PKok'), contentType: 'application/octet-stream' }
254250
})
255251

256252
const response = await GET(
@@ -259,24 +255,18 @@ describe('workspace files download route', () => {
259255
)
260256

261257
expect(response.status).toBe(500)
262-
// Reads already in flight finish, but the queued remainder is never started.
263258
expect(mockFetchServableWorkspaceFileBuffer.mock.calls.length).toBeLessThan(files.length)
264259
})
265260

266-
it('surfaces a storage failure as a 500 even when another document is pending', async () => {
261+
it('rejects a selection whose declared sizes already exceed the limit', async () => {
267262
mockListWorkspaceFiles.mockResolvedValue([
268-
workspaceFile('f1', 'pending.docx', 'folder-1'),
269-
workspaceFile('f2', 'broken.txt', 'folder-1'),
263+
{ ...workspaceFile('f1', 'a.mp4'), size: 200 * MB },
264+
{ ...workspaceFile('f2', 'b.mp4'), size: 200 * MB },
270265
])
271-
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
272-
if (file.name === 'pending.docx')
273-
throw new DocCompileUserError('Document is still being generated')
274-
throw new Error('storage down')
275-
})
276266

277267
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
278268

279-
// A 409 would tell the client to retry something that can never succeed.
280-
expect(response.status).toBe(500)
269+
expect(response.status).toBe(400)
270+
expect(mockDownloadFileStream).not.toHaveBeenCalled()
281271
})
282272
})

0 commit comments

Comments
 (0)