Skip to content

Commit b67b55a

Browse files
committed
perf(files): stream workspace archives instead of buffering them
The bulk download 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 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. The per-entry allowance and byte budget therefore govern documents only. archiver processes appended entries through a sequential queue; lazystream defers each storage read until that entry's turn, since handing the archiver an open stream per entry would hold more connections than the storage client pools. nodeReadableToWebStream moves out of input-validation.server.ts into a shared util rather than being written twice: Readable.toWeb throws ERR_INVALID_STATE when a consumer cancels while the source is still flowing, which is exactly what a cancelled download does. Trade: a storage read failing mid-archive truncates the response rather than returning 500, since the status is already sent. Documents cannot hit this. The table export route already behaves this way.
1 parent fdaaa05 commit b67b55a

6 files changed

Lines changed: 610 additions & 101 deletions

File tree

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { Readable } from 'stream'
5+
import { createMockRequest } from '@sim/testing'
6+
import JSZip from 'jszip'
7+
import { beforeEach, describe, expect, it, vi } from 'vitest'
8+
9+
const {
10+
mockGetSession,
11+
mockVerifyWorkspaceMembership,
12+
mockListWorkspaceFiles,
13+
mockListWorkspaceFileFolders,
14+
mockFetchServableWorkspaceFileBuffer,
15+
mockDownloadFileStream,
16+
} = vi.hoisted(() => ({
17+
mockGetSession: vi.fn(),
18+
mockVerifyWorkspaceMembership: vi.fn(),
19+
mockListWorkspaceFiles: vi.fn(),
20+
mockListWorkspaceFileFolders: vi.fn(),
21+
mockFetchServableWorkspaceFileBuffer: vi.fn(),
22+
mockDownloadFileStream: vi.fn(),
23+
}))
24+
25+
vi.mock('@/lib/auth', () => ({
26+
auth: { api: { getSession: vi.fn() } },
27+
getSession: mockGetSession,
28+
}))
29+
30+
vi.mock('@/app/api/workflows/utils', () => ({
31+
verifyWorkspaceMembership: mockVerifyWorkspaceMembership,
32+
}))
33+
34+
vi.mock('@/lib/uploads/contexts/workspace', () => ({
35+
listWorkspaceFiles: mockListWorkspaceFiles,
36+
listWorkspaceFileFolders: mockListWorkspaceFileFolders,
37+
buildWorkspaceFileFolderPathMap: (folders: Array<{ id: string; name: string }>) =>
38+
new Map(folders.map((folder) => [folder.id, folder.name])),
39+
fetchServableWorkspaceFileBuffer: mockFetchServableWorkspaceFileBuffer,
40+
}))
41+
42+
vi.mock('@/lib/uploads/core/storage-service', () => ({
43+
downloadFileStream: mockDownloadFileStream,
44+
}))
45+
46+
vi.mock('@sim/audit', () => ({
47+
recordAudit: vi.fn(),
48+
AuditAction: { FILE_DOWNLOADED: 'file.downloaded' },
49+
AuditResourceType: { FILE: 'file' },
50+
}))
51+
52+
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: vi.fn() }))
53+
54+
import { DocCompileUserError } from '@/lib/copilot/tools/server/files/doc-compile'
55+
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
56+
import { GET } from '@/app/api/workspaces/[id]/files/download/route'
57+
58+
const WORKSPACE_ID = 'ws-1'
59+
const context = { params: Promise.resolve({ id: WORKSPACE_ID }) }
60+
const MB = 1024 * 1024
61+
62+
function workspaceFile(id: string, name: string, folderId: string | null = 'folder-1') {
63+
return {
64+
id,
65+
name,
66+
key: `workspace/${WORKSPACE_ID}/${id}`,
67+
path: `/serve/${id}`,
68+
size: 100,
69+
type: 'application/octet-stream',
70+
folderId,
71+
}
72+
}
73+
74+
function requestFor(query: string) {
75+
return createMockRequest(
76+
'GET',
77+
undefined,
78+
{},
79+
`http://localhost:3000/api/workspaces/${WORKSPACE_ID}/files/download?${query}`
80+
)
81+
}
82+
83+
async function zipFrom(response: Response) {
84+
return JSZip.loadAsync(Buffer.from(await response.arrayBuffer()))
85+
}
86+
87+
describe('workspace files download route', () => {
88+
beforeEach(() => {
89+
vi.clearAllMocks()
90+
mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
91+
mockVerifyWorkspaceMembership.mockResolvedValue({ role: 'member' })
92+
mockListWorkspaceFileFolders.mockResolvedValue([
93+
{ id: 'folder-1', name: 'Reports', parentId: null },
94+
])
95+
mockDownloadFileStream.mockImplementation(async () => Readable.from([Buffer.from('plain')]))
96+
})
97+
98+
it('zips the rendered bytes for a generated doc, not its stored source', async () => {
99+
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'overview.docx')])
100+
// A real .docx is a ZIP; the stored source would be plain JS text.
101+
const rendered = Buffer.from('PKrendered-docx')
102+
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
103+
buffer: rendered,
104+
contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
105+
})
106+
107+
const response = await GET(requestFor('fileIds=f1'), context)
108+
109+
expect(response.status).toBe(200)
110+
const entry = (await zipFrom(response)).file('Reports/overview.docx')
111+
expect(entry).not.toBeNull()
112+
expect(Buffer.from(await entry!.async('uint8array'))).toEqual(rendered)
113+
})
114+
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+
156+
it('returns 409 naming the documents whose artifacts are still compiling', async () => {
157+
mockListWorkspaceFiles.mockResolvedValue([
158+
workspaceFile('f1', 'ready.docx'),
159+
workspaceFile('f2', 'pending.docx'),
160+
])
161+
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
162+
if (file.name === 'pending.docx')
163+
throw new DocCompileUserError('Document is still being generated')
164+
return { buffer: Buffer.from('PKok'), contentType: 'application/octet-stream' }
165+
})
166+
167+
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
168+
169+
expect(response.status).toBe(409)
170+
const body = await response.json()
171+
expect(body.error).toContain('pending.docx')
172+
expect(body.error).not.toContain('ready.docx')
173+
})
174+
175+
it('rejects with 400, not 500, when a document blows its own allowance', async () => {
176+
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'huge.docx')])
177+
mockFetchServableWorkspaceFileBuffer.mockRejectedValue(
178+
new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
179+
)
180+
181+
const response = await GET(requestFor('fileIds=f1'), context)
182+
183+
expect(response.status).toBe(400)
184+
const body = await response.json()
185+
expect(body.error).toContain('huge.docx')
186+
expect(body.error).not.toContain('Selected files total')
187+
})
188+
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+
})
201+
202+
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
203+
204+
expect(response.status).toBe(400)
205+
const body = await response.json()
206+
expect(body.error).toContain('Selected files total')
207+
expect(body.error).not.toContain('second.docx')
208+
})
209+
210+
it('lets an uploaded office file larger than the render headroom through', async () => {
211+
const big = { ...workspaceFile('f1', 'deck.pptx'), size: 80 * MB }
212+
mockListWorkspaceFiles.mockResolvedValue([big])
213+
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
214+
buffer: Buffer.from('PKdeck'),
215+
contentType: 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
216+
})
217+
218+
const response = await GET(requestFor('fileIds=f1'), context)
219+
220+
expect(response.status).toBe(200)
221+
// Capped at the declared size, not the smaller render headroom.
222+
expect(mockFetchServableWorkspaceFileBuffer.mock.calls[0][1].maxBytes).toBe(80 * MB)
223+
})
224+
225+
it('surfaces a storage failure as a 500 even when another document is pending', async () => {
226+
mockListWorkspaceFiles.mockResolvedValue([
227+
workspaceFile('f1', 'pending.docx'),
228+
workspaceFile('f2', 'broken.docx'),
229+
])
230+
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
231+
if (file.name === 'pending.docx')
232+
throw new DocCompileUserError('Document is still being generated')
233+
throw new Error('storage down')
234+
})
235+
236+
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
237+
238+
// A 409 would tell the client to retry something that can never succeed.
239+
expect(response.status).toBe(500)
240+
})
241+
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`)
245+
)
246+
mockListWorkspaceFiles.mockResolvedValue(files)
247+
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
248+
if (file.name === 'doc0.docx') throw new Error('storage down')
249+
return { buffer: Buffer.from('PKok'), contentType: 'application/octet-stream' }
250+
})
251+
252+
const response = await GET(
253+
requestFor(files.map((file) => `fileIds=${file.id}`).join('&')),
254+
context
255+
)
256+
257+
expect(response.status).toBe(500)
258+
expect(mockFetchServableWorkspaceFileBuffer.mock.calls.length).toBeLessThan(files.length)
259+
})
260+
261+
it('rejects a selection whose declared sizes already exceed the limit', async () => {
262+
mockListWorkspaceFiles.mockResolvedValue([
263+
{ ...workspaceFile('f1', 'a.mp4'), size: 200 * MB },
264+
{ ...workspaceFile('f2', 'b.mp4'), size: 200 * MB },
265+
])
266+
267+
const response = await GET(requestFor('fileIds=f1&fileIds=f2'), context)
268+
269+
expect(response.status).toBe(400)
270+
expect(mockDownloadFileStream).not.toHaveBeenCalled()
271+
})
272+
})

0 commit comments

Comments
 (0)