Skip to content

Commit 75dacb4

Browse files
committed
fix(files): probe served bytes before handing off, keep generated documents in-page, version local objects by inode
1 parent fc1e8ea commit 75dacb4

5 files changed

Lines changed: 44 additions & 21 deletions

File tree

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -449,15 +449,19 @@ function StreamedMediaPlayer({
449449
const diagnosing = useRef(false)
450450

451451
/**
452-
* A media element reports only that it failed, not why, so the URL is asked directly. A 404 means
453-
* the key was replaced; the record is re-resolved, and a moved key re-renders this preview at the
454-
* new URL. Bytes that are there but would not play are the browser's format support.
452+
* A media element reports only that it failed, not why, so the URL is asked directly for its
453+
* first byte, which opens storage. A 404 means the key was replaced; the record is re-resolved,
454+
* and a moved key re-renders this preview at the new URL. Bytes that are there but would not play
455+
* are the browser's format support.
455456
*/
456457
const handleError = useCallback(async () => {
457458
if (diagnosing.current) return
458459
diagnosing.current = true
459-
// boundary-raw-fetch: HEAD probe of the binary serve path, which has no contract
460-
const probe = await fetch(src, { method: 'HEAD', cache: 'no-store' }).catch(() => null)
460+
// boundary-raw-fetch: ranged probe of the binary serve path, which has no contract
461+
const probe = await fetch(src, { headers: { Range: 'bytes=0-0' }, cache: 'no-store' }).catch(
462+
() => null
463+
)
464+
await probe?.body?.cancel()
461465
if (probe?.status === 404) await reresolveRecord()
462466
setStatus(probe?.ok ? 'unplayable' : 'unavailable')
463467
}, [src, reresolveRecord])

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ describe('large file downloads', () => {
149149
await triggerFileDownload(video)
150150

151151
expect(fetchMock).toHaveBeenCalledOnce()
152-
expect(fetchMock.mock.calls[0]?.[1]).toMatchObject({ method: 'HEAD' })
152+
expect(fetchMock.mock.calls[0]?.[1]).toMatchObject({ headers: { Range: 'bytes=0-0' } })
153153
expect(createObjectURL).not.toHaveBeenCalled()
154154
expect(downloadedHref).toMatch(/^\/api\/files\/serve\/workspace%2Fworkspace-1%2Fclip\.mp4\?/)
155155
expect(downloadedName).toBe('clip.mp4')
@@ -165,7 +165,15 @@ describe('large file downloads', () => {
165165
it('keeps fetching a file small enough to hold, so a rejection still surfaces as an error', async () => {
166166
await triggerFileDownload({ ...video, size: 1024 })
167167

168-
expect(fetchMock.mock.calls[0]?.[1]).not.toMatchObject({ method: 'HEAD' })
168+
expect(fetchMock.mock.calls[0]?.[1]).not.toHaveProperty('headers')
169+
expect(createObjectURL).toHaveBeenCalledOnce()
170+
})
171+
172+
it('keeps fetching a large generated document, which must compile before it can be served', async () => {
173+
await triggerFileDownload({ ...video, name: 'report.pdf', type: 'text/x-pdflibjs' })
174+
175+
expect(fetchMock).toHaveBeenCalledOnce()
176+
expect(fetchMock.mock.calls[0]?.[1]).not.toHaveProperty('headers')
169177
expect(createObjectURL).toHaveBeenCalledOnce()
170178
})
171179
})

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { requestRaw } from '@/lib/api/client/request'
33
import { downloadWorkspaceFileItemsContract } from '@/lib/api/contracts/workspace-file-folders'
44
import { exportWorkspaceFileSnapshotContract } from '@/lib/api/contracts/workspace-files'
55
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
6-
import { isMarkdownFile } from '@/lib/uploads/utils/file'
6+
import { isMarkdownFile, needsRenderedArtifact } from '@/lib/uploads/utils/file'
77
import { normalizeMimeType } from '@/lib/uploads/utils/mime'
88

99
/** Action-time content from the mounted viewer, scoped so another file cannot consume it. */
@@ -14,11 +14,11 @@ export interface FileDownloadSource {
1414
}
1515

1616
/**
17-
* Above this a file is handed to the browser's download manager instead of being read into the
18-
* page. Holding it as a `Blob` costs its whole size in tab memory and shows no progress until the
19-
* last byte lands; the download manager streams it to disk. Below it, fetching first keeps a
20-
* rejection — a document still compiling — on screen as an error rather than as a failed entry
21-
* in the browser's downloads list.
17+
* Above this a stored file is handed to the browser's download manager instead of being read into
18+
* the page. Holding it as a `Blob` costs its whole size in tab memory and shows no progress until
19+
* the last byte lands; the download manager streams it to disk. Below it, and for a generated
20+
* document, which must compile first, fetching keeps a rejection on screen as an error rather than
21+
* as a failed entry in the browser's downloads list.
2222
*/
2323
const IN_PAGE_DOWNLOAD_MAX_BYTES = 100 * 1024 * 1024
2424

@@ -97,9 +97,16 @@ export async function triggerFileDownload(
9797
? `/api/files/export/${encodeURIComponent(record.id)}`
9898
: `/api/files/serve/${encodeURIComponent(record.key)}?context=${record.storageContext ?? 'workspace'}&t=${Date.now()}`
9999

100-
if (!exportsMarkdown && record.size > IN_PAGE_DOWNLOAD_MAX_BYTES) {
101-
// boundary-raw-fetch: HEAD probe of a binary path with no contract, so a refusal still surfaces here
102-
const probe = await fetch(url, { method: 'HEAD', cache: 'no-store' })
100+
if (
101+
!exportsMarkdown &&
102+
!needsRenderedArtifact(record.type, record.name) &&
103+
record.size > IN_PAGE_DOWNLOAD_MAX_BYTES
104+
) {
105+
// A first-byte read opens storage itself, so a missing object is refused here, in place,
106+
// rather than as a failed entry in the browser's downloads list.
107+
// boundary-raw-fetch: ranged probe of a binary path with no contract
108+
const probe = await fetch(url, { headers: { Range: 'bytes=0-0' }, cache: 'no-store' })
109+
await probe.body?.cancel()
103110
if (!probe.ok) throw new Error(`Failed to download "${record.name}"`)
104111
clickDownloadLink(url, record.name)
105112
return

‎apps/sim/lib/uploads/core/storage-service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -808,8 +808,8 @@ export async function headObject(
808808
const { UPLOAD_DIR_SERVER } = await getLocalStorageSetup()
809809
try {
810810
const file = await stat(join(UPLOAD_DIR_SERVER, sanitizeFileKey(key)))
811-
// A local write replaces the file in place, so its modification time is what changes with it.
812-
return { size: file.size, version: `${file.mtimeMs}` }
811+
const { localObjectVersion } = await import('@/lib/uploads/upload-session/provider')
812+
return { size: file.size, version: localObjectVersion(file) }
813813
} catch (error) {
814814
const code = (error as NodeJS.ErrnoException).code
815815
if (code === 'ENOENT') return null

‎apps/sim/lib/uploads/upload-session/provider.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ async function headLocalObject(key: string): Promise<UploadObjectHead | null> {
790790
size: file.size,
791791
contentType: metadata.contentType,
792792
uploadId: metadata.uploadId,
793-
version: localVersion(file),
793+
version: localObjectVersion(file),
794794
}
795795
}
796796

@@ -803,7 +803,7 @@ async function deleteLocalObjectVersion(key: string, version: string): Promise<v
803803
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return
804804
throw error
805805
}
806-
if (localVersion(current) !== version) return
806+
if (localObjectVersion(current) !== version) return
807807
await unlink(path)
808808
await rm(localMetadataPath(key), { force: true })
809809
await rmdir(dirname(path)).catch((error: NodeJS.ErrnoException) => {
@@ -841,7 +841,11 @@ async function readLocalMetadata(path: string): Promise<LocalUploadMetadata> {
841841
}
842842
}
843843

844-
function localVersion(file: Awaited<ReturnType<typeof stat>>): string {
844+
/**
845+
* Version of a local upload object: what changes whenever its bytes do, including a same-size
846+
* replacement within one timestamp tick.
847+
*/
848+
export function localObjectVersion(file: Awaited<ReturnType<typeof stat>>): string {
845849
return `${file.dev}:${file.ino}:${file.size}:${file.mtimeMs}`
846850
}
847851

0 commit comments

Comments
 (0)