Skip to content

Commit fc1e8ea

Browse files
committed
fix(files): stream finished document uploads, diagnose playback errors, prove ranges on real bytes
1 parent 9873df0 commit fc1e8ea

5 files changed

Lines changed: 55 additions & 26 deletions

File tree

‎apps/sim/app/api/files/serve/[...path]/route.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,19 @@ describe('File Serve API Route', () => {
520520
expect(response.headers.get('Content-Type')).toBe('text/csv')
521521
})
522522

523+
it('streams an uploaded document, which is already the finished file', async () => {
524+
const response = await serveWorkspaceFile({
525+
...video,
526+
name: 'scan.pdf',
527+
key: 'workspace/test-workspace-id/scan.pdf',
528+
type: 'application/pdf',
529+
})
530+
531+
expect(response.status).toBe(200)
532+
expect(response.headers.get('Content-Length')).toBe(String(video.size))
533+
expect(fileUtilsServerMockFns.mockDownloadServableFileFromStorage).not.toHaveBeenCalled()
534+
})
535+
523536
it('streams the raw source of a page rather than rendering it', async () => {
524537
mockGetContentType.mockReturnValue('text/html')
525538
const response = await serveWorkspaceFile(

‎apps/sim/app/api/files/serve/[...path]/route.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import { resolveStoredFileContext } from '@/lib/uploads/server/metadata'
3232
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
3333
import {
3434
inferContextFromKey,
35-
isRenderableDocumentName,
35+
needsRenderedArtifact,
3636
resolveEffectiveMimeType,
3737
} from '@/lib/uploads/utils/file'
3838
import { internalWorkspaceFileServeAuth } from '@/lib/workspace-files/api'
@@ -218,7 +218,9 @@ function servesStoredObject(
218218
if (recordedType === SIM_PAGE_CONTENT_TYPE || filename.toLowerCase().endsWith('.html')) {
219219
return false
220220
}
221-
return !isRenderableDocumentName(filename)
221+
// A recorded type tells a finished upload from a generation source; without one, every
222+
// document name may be a source.
223+
return !needsRenderedArtifact(recordedType, filename)
222224
}
223225

224226
/**

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ function StreamedMediaPlayer({
496496
loading={status === 'loading'}
497497
mediaRef={syncWithElement}
498498
onLoadedMetadata={() => setStatus('ready')}
499-
onError={status === 'loading' ? handleError : undefined}
499+
onError={handleError}
500500
/>
501501
)
502502
}

‎apps/sim/lib/uploads/client/download.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ describe('large file downloads', () => {
134134

135135
beforeEach(() => {
136136
downloadedHref = ''
137-
vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation(function () {
137+
vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation(function (
138+
this: HTMLAnchorElement
139+
) {
138140
downloadedHref = this.getAttribute('href') ?? ''
139141
downloadedName = this.download
140142
click()
Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
import { Readable } from 'node:stream'
2-
import { storageServiceMock, storageServiceMockFns } from '@sim/testing'
1+
import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
2+
import { tmpdir } from 'node:os'
3+
import { dirname, join } from 'node:path'
34
import {
45
createDelegatedPrincipal,
56
createSessionPrincipal,
67
} from '@sim/testing/factories/principal.factory'
78
import { fileReadReceiptMock } from '@sim/testing/mocks/file-read-receipt.mock'
9+
import { uploadsConfigMock, uploadsConfigMockFns } from '@sim/testing/mocks/uploads-config.mock'
10+
import { uploadsMetadataMock } from '@sim/testing/mocks/uploads-metadata.mock'
11+
import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock'
812
import {
913
workspaceFileSecretProvenanceMock,
1014
workspaceFileSecretProvenanceMockFns,
@@ -22,60 +26,65 @@ vi.mock(
2226
'@/lib/uploads/contexts/workspace/file-secret-provenance',
2327
() => workspaceFileSecretProvenanceMock
2428
)
25-
vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock)
29+
vi.mock('@/lib/uploads/config', () => uploadsConfigMock)
30+
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
31+
vi.mock('@/lib/uploads/server/metadata', () => uploadsMetadataMock)
2632

2733
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
2834
import { streamAuthorizedWorkspaceFile } from '@/lib/workspace-files/application/stream-authorized-workspace-file'
2935

36+
const CONTENT = Buffer.from('0123456789abcdefghij')
37+
3038
const file = {
3139
id: 'file-1',
3240
workspaceId: 'workspace-1',
3341
name: 'clip.mp4',
3442
key: 'workspace/workspace-1/clip.mp4',
3543
path: '/api/files/serve/workspace/workspace-1/clip.mp4',
36-
size: 6,
44+
size: CONTENT.length,
3745
type: 'video/mp4',
3846
uploadedBy: 'user-1',
3947
uploadedAt: new Date('2026-08-01T00:00:00Z'),
4048
updatedAt: new Date('2026-08-01T00:00:00Z'),
4149
} satisfies WorkspaceFileRecord
4250

43-
let source: Readable
44-
45-
async function readAll(stream: ReadableStream<Uint8Array>): Promise<string> {
46-
return Buffer.from(await new Response(stream).arrayBuffer()).toString()
51+
async function readAll(stream: ReadableStream<Uint8Array>): Promise<Buffer> {
52+
return Buffer.from(await new Response(stream).arrayBuffer())
4753
}
4854

4955
describe('streamAuthorizedWorkspaceFile', () => {
5056
beforeEach(() => {
51-
source = Readable.from([Buffer.from('stream')])
52-
storageServiceMockFns.mockDownloadFileStream.mockResolvedValue(source)
57+
const uploads = mkdtempSync(join(tmpdir(), 'stream-authorized-'))
58+
mkdirSync(dirname(join(uploads, file.key)), { recursive: true })
59+
writeFileSync(join(uploads, file.key), CONTENT)
60+
setUploadDirServer(uploads)
61+
uploadsConfigMockFns.mockGetStorageConfig.mockReturnValue({ bucket: 'b', region: 'r' })
5362
})
5463

55-
it('streams the stored bytes once access still holds after storage opens', async () => {
64+
it('streams every stored byte once access still holds after storage opens', async () => {
5665
const stream = await streamAuthorizedWorkspaceFile({
5766
file,
5867
principal: createSessionPrincipal(),
5968
authorize: async () => undefined,
6069
})
61-
await expect(readAll(stream)).resolves.toBe('stream')
70+
71+
expect(await readAll(stream)).toEqual(CONTENT)
6272
})
6373

64-
it('reads only the requested span from storage', async () => {
65-
await streamAuthorizedWorkspaceFile({
74+
it('reads exactly the requested inclusive span and nothing else', async () => {
75+
const stream = await streamAuthorizedWorkspaceFile({
6676
file,
6777
principal: createSessionPrincipal(),
6878
authorize: async () => undefined,
69-
range: { start: 2, end: 4 },
70-
})
71-
expect(storageServiceMockFns.mockDownloadFileStream).toHaveBeenCalledWith({
72-
key: file.key,
73-
context: 'workspace',
74-
range: { start: 2, end: 4 },
79+
range: { start: 5, end: 9 },
7580
})
81+
82+
expect((await readAll(stream)).toString()).toBe('56789')
7683
})
7784

7885
it('disposes the opened stream when access is revoked before it is handed out', async () => {
86+
const cancel = vi.spyOn(ReadableStream.prototype, 'cancel')
87+
7988
await expect(
8089
streamAuthorizedWorkspaceFile({
8190
file,
@@ -85,7 +94,8 @@ describe('streamAuthorizedWorkspaceFile', () => {
8594
},
8695
})
8796
).rejects.toThrow('revoked')
88-
expect(source.destroyed).toBe(true)
97+
expect(cancel).toHaveBeenCalled()
98+
cancel.mockRestore()
8999
})
90100

91101
it('refuses and disposes the stream when provenance changes after storage opens', async () => {
@@ -94,6 +104,7 @@ describe('streamAuthorizedWorkspaceFile', () => {
94104
{ status: 'exact', entries: [] }
95105
)
96106
vi.mocked(recheckFileReadReceipt).mockResolvedValueOnce({ status: 'unknown' })
107+
const cancel = vi.spyOn(ReadableStream.prototype, 'cancel')
97108

98109
await expect(
99110
observeContentDelivery(
@@ -102,6 +113,7 @@ describe('streamAuthorizedWorkspaceFile', () => {
102113
streamAuthorizedWorkspaceFile({ file, principal: chat, authorize: async () => undefined })
103114
)
104115
).rejects.toBeInstanceOf(ContentRefusedError)
105-
expect(source.destroyed).toBe(true)
116+
expect(cancel).toHaveBeenCalled()
117+
cancel.mockRestore()
106118
})
107119
})

0 commit comments

Comments
 (0)