Skip to content

Commit 5d3107f

Browse files
committed
fix(files): finish the render cancellation and failure-surfacing edges
- Race the E2B render against the caller's signal too. Only the isolated-vm branch did, so an aborted request on the E2B path waited for the sandbox to finish and could return a success the caller no longer wanted. - Attach a terminal handler to the shared render. Every caller races it against its own signal, so all of them can walk away; a later rejection with no waiters left would otherwise surface as an unhandled rejection. - Stop narrowing what throwOnDocNotReady rethrows. readUserFileContent now runs document compiles and can fail in ways this module has no business enumerating; narrowing produced three consecutive review rounds of "this particular failure is still swallowed". The flag means "do not degrade". - Do not mark an unrendered response immutable. The serve route caches versioned responses for a year, which would pin a one-off render failure to that URL long after a later compile succeeds on the same version.
1 parent 78c16ec commit 5d3107f

3 files changed

Lines changed: 35 additions & 37 deletions

File tree

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

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ async function compileDocumentIfNeeded(
3737
raw: boolean,
3838
ownerKey: string | undefined,
3939
signal: AbortSignal | undefined
40-
): Promise<{ buffer: Buffer; contentType: string }> {
40+
): Promise<{ buffer: Buffer; contentType: string; unrendered?: boolean }> {
4141
if (raw) return { buffer, contentType: getContentType(filename) }
4242
return resolveServableDocBytes({
4343
rawBuffer: buffer,
@@ -67,6 +67,10 @@ const WORKSPACE_REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
6767
* bumps on every edit — so the browser may cache it indefinitely; re-opens and
6868
* focus refetches then resolve from cache with no round trip. Unversioned workspace
6969
* reads stay revalidated because the same storage key is edited in place.
70+
*
71+
* Callers pass `versioned && !unrendered`: a render that failed returns the stored
72+
* bytes as opaque data, and marking that immutable for a year would pin the failure
73+
* to the URL long after a later compile succeeds on the same version.
7074
*/
7175
function resolveServeCacheControl(
7276
versioned: boolean,
@@ -195,22 +199,19 @@ async function handleLocalFile(
195199
const segment = filename.split('/').pop() || filename
196200
const displayName = stripStorageKeyPrefix(segment)
197201
const workspaceId = getWorkspaceIdForCompile(filename)
198-
const { buffer: fileBuffer, contentType } = await compileDocumentIfNeeded(
199-
rawBuffer,
200-
displayName,
201-
workspaceId,
202-
raw,
203-
ownerKey,
204-
signal
205-
)
202+
const {
203+
buffer: fileBuffer,
204+
contentType,
205+
unrendered,
206+
} = await compileDocumentIfNeeded(rawBuffer, displayName, workspaceId, raw, ownerKey, signal)
206207

207208
logger.info('Local file served', { userId, filename, size: fileBuffer.length })
208209

209210
return createFileResponse({
210211
buffer: fileBuffer,
211212
contentType,
212213
filename: displayName,
213-
cacheControl: resolveServeCacheControl(versioned, contextParam),
214+
cacheControl: resolveServeCacheControl(versioned && !unrendered, contextParam),
214215
})
215216
} catch (error) {
216217
logger.error('Error reading local file:', error)
@@ -257,14 +258,11 @@ async function handleCloudProxy(
257258
const segment = cloudKey.split('/').pop() || 'download'
258259
const displayName = stripStorageKeyPrefix(segment)
259260
const workspaceId = getWorkspaceIdForCompile(cloudKey)
260-
const { buffer: fileBuffer, contentType } = await compileDocumentIfNeeded(
261-
rawBuffer,
262-
displayName,
263-
workspaceId,
264-
raw,
265-
ownerKey,
266-
signal
267-
)
261+
const {
262+
buffer: fileBuffer,
263+
contentType,
264+
unrendered,
265+
} = await compileDocumentIfNeeded(rawBuffer, displayName, workspaceId, raw, ownerKey, signal)
268266

269267
logger.info('Cloud file served', {
270268
userId,
@@ -277,7 +275,7 @@ async function handleCloudProxy(
277275
buffer: fileBuffer,
278276
contentType,
279277
filename: displayName,
280-
cacheControl: resolveServeCacheControl(versioned, context),
278+
cacheControl: resolveServeCacheControl(versioned && !unrendered, context),
281279
})
282280
} catch (error) {
283281
logger.error('Error downloading from cloud storage:', error)

apps/sim/lib/copilot/tools/server/files/doc-compile.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,11 @@ function coalesceRender(
541541
const existing = inFlightRenders.get(key)
542542
if (existing) return existing
543543
const started = run().finally(() => inFlightRenders.delete(key))
544+
// Every caller races this against its own signal, so all of them can walk away
545+
// before it settles. Attach a terminal handler so a later rejection with no
546+
// waiters left is not reported as an unhandled rejection — callers still observe
547+
// it through their own reference.
548+
started.catch(() => {})
544549
inFlightRenders.set(key, started)
545550
return started
546551
}
@@ -675,7 +680,11 @@ export async function resolveServableDocBytes(args: {
675680
// (content-addressed), so racing a still-running write-time compile is wasteful
676681
// but correct.
677682
try {
678-
return await coalesceRender(renderKey, () => compileDoc({ source, fileName, workspaceId }))
683+
// Same shape as the isolated-vm branch below: the shared run carries no
684+
// caller's signal, and each caller races its own so an aborting reader gives
685+
// up promptly without cancelling the render for everyone else.
686+
const shared = coalesceRender(renderKey, () => compileDoc({ source, fileName, workspaceId }))
687+
return await (signal ? Promise.race([shared, rejectOnAbort(signal)]) : shared)
679688
} catch (error) {
680689
// Only a script error is deterministic — the same bytes will never render, so
681690
// remembering that is safe. Infra failures (sandbox create/timeout, S3, an

apps/sim/lib/uploads/utils/user-file-base64.server.ts

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import {
2727
ExecutionResourceLimitError,
2828
isExecutionResourceLimitError,
2929
} from '@/lib/execution/resource-errors'
30-
import { UnrenderableDocumentError } from '@/lib/uploads/utils/file-utils'
3130
import type { UserFile } from '@/executor/types'
3231

3332
const INLINE_BASE64_JSON_OVERHEAD_BYTES = 512 * 1024
@@ -444,23 +443,15 @@ async function resolveBase64(
444443
// already-finished result opt out, so a late compile cannot retroactively
445444
// fail completed work.
446445
if (options.throwOnDocNotReady) {
447-
// This caller cannot use a file without content, so any reason the bytes are
448-
// missing must reach it verbatim. Degrading to null here is what produced the
449-
// misleading "may exceed size limit or no longer accessible" message for a
450-
// document that was actually still compiling, or one whose stored bytes are
451-
// not the format its name claims.
446+
// This caller cannot use a file without content, so every failure reaches it
447+
// verbatim — still compiling, unrenderable, a sandbox outage, a storage error.
452448
//
453-
// Imported lazily: `servable-file-response` pulls in the doc-compile module
454-
// graph (remote sandbox, sandbox task runner, execution limits), and a static
455-
// import here would load all of it for every hydration consumer — mirroring
456-
// the deliberate dynamic import in file-utils.server.ts.
457-
if (error instanceof UnrenderableDocumentError) {
458-
throw error
459-
}
460-
const { isDocNotReadyError } = await import('@/lib/uploads/utils/servable-file-response')
461-
if (isDocNotReadyError(error)) {
462-
throw error
463-
}
449+
// Deliberately not narrowed to specific error classes. Doing that produced
450+
// three consecutive rounds of "this particular failure is still swallowed",
451+
// because `readUserFileContent` now runs document compiles and can fail in
452+
// ways this module has no business enumerating. The flag means "do not
453+
// degrade", not "do not degrade for the failures we thought of".
454+
throw error
464455
}
465456
logger.warn(`[${requestId}] Failed to hydrate base64 for ${file.name}`, error)
466457
return null

0 commit comments

Comments
 (0)