Skip to content

Commit 8d567de

Browse files
committed
improvement(files): simplify the archive route and stop buffering real uploads
Four review passes over the branch converged on the same points. Routing every office extension through the buffered path held an entire selection of genuinely uploaded documents in memory — for a check the resolver settles on the first few magic bytes. Only files stored under a generator-source content type need resolving; a real .docx serves what is stored and now streams like anything else, which is what the change was for. With resolution sequential, the AbortController cancelled nothing (its signal never reached the storage read), the success-path over-limit branch was unreachable, and the cancellation guard in the catch could not fire. A plain loop that returns at the point of failure replaces the outcome record, the sentinel, the fan-out helper at limit 1, and four post-hoc scans. ZIP_MATERIALIZE_CONCURRENCY was dead, and the aggregate over-limit message reported a byte count that was by construction under the limit. lazystream and its types are gone: Readable.from over an async generator defers the open the same way and propagates source errors natively, so the PassThrough relay went with them. The client uses requestRaw with the existing binary contract instead of a hand-built query string and a re-typed error extractor, and both archive call sites share one callback. The render headroom moves beside isRenderableDocumentName, the servable reader stops minting a request id per file, and the v1 download returns a view rather than a second full copy.
1 parent 8828e1e commit 8d567de

9 files changed

Lines changed: 198 additions & 243 deletions

File tree

apps/sim/app/api/v1/files/[fileId]/route.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,24 @@ export const GET = withRouteHandler(async (request: NextRequest, context: FileRo
7979
{ groups: { workspace: workspaceId } }
8080
)
8181

82-
return new Response(new Uint8Array(buffer), {
83-
status: 200,
84-
headers: {
85-
'Content-Type': contentType || fileRecord.type || 'application/octet-stream',
86-
'Content-Disposition': `attachment; filename="${fileRecord.name.replace(/[^\w.-]/g, '_')}"; filename*=UTF-8''${encodeURIComponent(fileRecord.name)}`,
87-
'Content-Length': String(buffer.length),
88-
'X-File-Id': fileRecord.id,
89-
'X-File-Name': encodeURIComponent(fileRecord.name),
90-
'X-Uploaded-At':
91-
fileRecord.uploadedAt instanceof Date
92-
? fileRecord.uploadedAt.toISOString()
93-
: String(fileRecord.uploadedAt),
94-
},
95-
})
82+
// View, not copy — a second full copy would double peak memory for a large file.
83+
return new Response(
84+
new Uint8Array(buffer.buffer as ArrayBuffer, buffer.byteOffset, buffer.byteLength),
85+
{
86+
status: 200,
87+
headers: {
88+
'Content-Type': contentType || fileRecord.type || 'application/octet-stream',
89+
'Content-Disposition': `attachment; filename="${fileRecord.name.replace(/[^\w.-]/g, '_')}"; filename*=UTF-8''${encodeURIComponent(fileRecord.name)}`,
90+
'Content-Length': String(buffer.length),
91+
'X-File-Id': fileRecord.id,
92+
'X-File-Name': encodeURIComponent(fileRecord.name),
93+
'X-Uploaded-At':
94+
fileRecord.uploadedAt instanceof Date
95+
? fileRecord.uploadedAt.toISOString()
96+
: String(fileRecord.uploadedAt),
97+
},
98+
}
99+
)
96100
} catch (error) {
97101
// A generated doc whose artifact is still compiling is retryable, not a fault:
98102
// without this the caller sees a 500 and has no reason to try again.

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

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ function workspaceFile(id: string, name: string, folderId: string | null = 'fold
7171
}
7272
}
7373

74+
/** A file whose stored bytes are a generator source, so it must be resolved. */
75+
function generatedDocument(id: string, name: string, folderId: string | null = 'folder-1') {
76+
return { ...workspaceFile(id, name, folderId), type: 'text/x-docxjs' }
77+
}
78+
7479
function requestFor(query: string) {
7580
return createMockRequest(
7681
'GET',
@@ -96,7 +101,7 @@ describe('workspace files download route', () => {
96101
})
97102

98103
it('zips the rendered bytes for a generated doc, not its stored source', async () => {
99-
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'overview.docx')])
104+
mockListWorkspaceFiles.mockResolvedValue([generatedDocument('f1', 'overview.docx')])
100105
// A real .docx is a ZIP; the stored source would be plain JS text.
101106
const rendered = Buffer.from('PKrendered-docx')
102107
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
@@ -139,7 +144,7 @@ describe('workspace files download route', () => {
139144
{ id: 'folder-2', name: 'visuals', parentId: 'folder-1' },
140145
])
141146
mockListWorkspaceFiles.mockResolvedValue([
142-
workspaceFile('f1', 'summary.docx', 'folder-1'),
147+
generatedDocument('f1', 'summary.docx', 'folder-1'),
143148
workspaceFile('f2', 'hero.png', 'folder-2'),
144149
])
145150
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
@@ -155,8 +160,8 @@ describe('workspace files download route', () => {
155160

156161
it('returns 409 naming the documents whose artifacts are still compiling', async () => {
157162
mockListWorkspaceFiles.mockResolvedValue([
158-
workspaceFile('f1', 'ready.docx'),
159-
workspaceFile('f2', 'pending.docx'),
163+
generatedDocument('f1', 'ready.docx'),
164+
generatedDocument('f2', 'pending.docx'),
160165
])
161166
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
162167
if (file.name === 'pending.docx')
@@ -173,7 +178,7 @@ describe('workspace files download route', () => {
173178
})
174179

175180
it('rejects with 400, not 500, when a document blows its own allowance', async () => {
176-
mockListWorkspaceFiles.mockResolvedValue([workspaceFile('f1', 'huge.docx')])
181+
mockListWorkspaceFiles.mockResolvedValue([generatedDocument('f1', 'huge.docx')])
177182
mockFetchServableWorkspaceFileBuffer.mockRejectedValue(
178183
new PayloadSizeLimitError({ label: 'servable file download', maxBytes: 1 })
179184
)
@@ -188,8 +193,8 @@ describe('workspace files download route', () => {
188193

189194
it('blames the shared budget once earlier documents have consumed it', async () => {
190195
mockListWorkspaceFiles.mockResolvedValue([
191-
workspaceFile('f1', 'first.docx'),
192-
workspaceFile('f2', 'second.docx'),
196+
generatedDocument('f1', 'first.docx'),
197+
generatedDocument('f2', 'second.docx'),
193198
])
194199
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
195200
// The first document eats the whole budget, so the second's cap is the remainder.
@@ -207,25 +212,40 @@ describe('workspace files download route', () => {
207212
expect(body.error).not.toContain('second.docx')
208213
})
209214

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-
})
215+
it('streams an uploaded office file rather than resolving it', async () => {
216+
// A real upload serves exactly its stored bytes, so it must not take the buffered
217+
// path — otherwise a selection of large decks is held in memory for nothing.
218+
const upload = {
219+
...workspaceFile('f1', 'deck.pptx'),
220+
size: 80 * MB,
221+
type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
222+
}
223+
mockListWorkspaceFiles.mockResolvedValue([upload])
217224

218225
const response = await GET(requestFor('fileIds=f1'), context)
226+
await zipFrom(response)
219227

220228
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)
229+
expect(mockFetchServableWorkspaceFileBuffer).not.toHaveBeenCalled()
230+
expect(mockDownloadFileStream).toHaveBeenCalledTimes(1)
231+
})
232+
233+
it('caps a generated document at the render headroom', async () => {
234+
mockListWorkspaceFiles.mockResolvedValue([generatedDocument('f1', 'report.docx')])
235+
mockFetchServableWorkspaceFileBuffer.mockResolvedValue({
236+
buffer: Buffer.from('PKdoc'),
237+
contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
238+
})
239+
240+
await GET(requestFor('fileIds=f1'), context)
241+
242+
expect(mockFetchServableWorkspaceFileBuffer.mock.calls[0][1].maxBytes).toBe(50 * MB)
223243
})
224244

225245
it('surfaces a storage failure as a 500 even when another document is pending', async () => {
226246
mockListWorkspaceFiles.mockResolvedValue([
227-
workspaceFile('f1', 'pending.docx'),
228-
workspaceFile('f2', 'broken.docx'),
247+
generatedDocument('f1', 'pending.docx'),
248+
generatedDocument('f2', 'broken.docx'),
229249
])
230250
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {
231251
if (file.name === 'pending.docx')
@@ -241,7 +261,7 @@ describe('workspace files download route', () => {
241261

242262
it('stops resolving documents once one hard-fails', async () => {
243263
const files = Array.from({ length: 20 }, (_, index) =>
244-
workspaceFile(`f${index}`, `doc${index}.docx`)
264+
generatedDocument(`f${index}`, `doc${index}.docx`)
245265
)
246266
mockListWorkspaceFiles.mockResolvedValue(files)
247267
mockFetchServableWorkspaceFileBuffer.mockImplementation(async (file: { name: string }) => {

0 commit comments

Comments
 (0)