fix(uploads): treat a missing storage object as absent metadata, not a failure

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

@@ -7,6 +7,13 @@ import { hybridAuthMockFns, storageServiceMock, storageServiceMockFns } from '@s
 import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
+vi.mock('@sim/logger', () => ({
+  createLogger: vi.fn(() => serveLogger),
+  logger: serveLogger,
+  runWithRequestContext: vi.fn(<T>(_ctx: unknown, fn: () => T): T => fn()),
+  getRequestContext: vi.fn(() => undefined),
+}))
+
 const {
   mockVerifyFileAccess,
   mockReadFile,
@@ -18,6 +25,7 @@ const {
   mockCreateFileResponse,
   mockCreateErrorResponse,
   FileNotFoundError,
+  serveLogger,
 } = vi.hoisted(() => {
   class FileNotFoundErrorClass extends Error {
     constructor(message: string) {
@@ -26,6 +34,7 @@ const {
     }
   }
   return {
+    serveLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
     mockVerifyFileAccess: vi.fn(),
     mockReadFile: vi.fn(),
     mockIsUsingCloudStorage: vi.fn(),
@@ -232,4 +241,32 @@ describe('File Serve API Route', () => {
       })
     }
   })
+
+  describe('failure log level', () => {
+    it('records a missing file at info, not error', async () => {
+      /** A superseded key is an ordinary 404, not a server fault. */
+      const req = new NextRequest('http://localhost:3000/api/files/serve/')
+      const response = await GET(req, { params: Promise.resolve({ path: [] }) })
+
+      expect(response.status).toBe(404)
+      expect(serveLogger.info).toHaveBeenCalledWith(
+        'Error serving file:',
+        expect.objectContaining({ reason: expect.any(String) })
+      )
+      expect(serveLogger.error).not.toHaveBeenCalled()
+    })
+
+    it('still records a genuine failure at error', async () => {
+      mockVerifyFileAccess.mockRejectedValueOnce(new Error('permission backend down'))
+
+      const req = new NextRequest(
+        'http://localhost:3000/api/files/serve/workspace/ws/test-file.txt'
+      )
+      await GET(req, {
+        params: Promise.resolve({ path: ['workspace', 'ws', 'test-file.txt'] }),
+      }).catch(() => undefined)
+
+      expect(serveLogger.error).toHaveBeenCalled()
+    })
+  })
 })

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

@@ -26,6 +26,23 @@ import {
 
 const logger = createLogger('FilesServeAPI')
 
+/**
+ * Records a failed serve at a level that matches whose fault it is.
+ *
+ * A file that is not there is an ordinary answer rather than a server fault: a
+ * workspace file is rewritten under a new key on every content update, so a reader
+ * holding the previous key lands here routinely and correctly receives a 404. Each
+ * handler rethrows into the outer one, so logging those at `error` reports the same
+ * expected 404 twice and buries the failures that do warrant attention.
+ */
+function logServeFailure(message: string, error: unknown): void {
+  if (error instanceof FileNotFoundError) {
+    logger.info(message, { reason: error.message })
+    return
+  }
+  logger.error(message, error)
+}
+
 interface ServeOptions {
   /** `raw=1` — bypass all resolution and serve the stored source as-is. */
   raw: boolean
@@ -179,7 +196,7 @@ export const GET = withRouteHandler(
         return NextResponse.json({ error: 'Document is still being generated' }, { status: 409 })
       }
 
-      logger.error('Error serving file:', error)
+      logServeFailure('Error serving file:', error)
 
       if (error instanceof FileNotFoundError) {
         return createErrorResponse(error)
@@ -244,7 +261,7 @@ async function handleLocalFile(
       cacheControl: resolveServeCacheControl(options.versioned, contextParam),
     })
   } catch (error) {
-    logger.error('Error reading local file:', error)
+    logServeFailure('Error reading local file:', error)
     throw error
   }
 }
@@ -311,7 +328,7 @@ async function handleCloudProxy(
       cacheControl: resolveServeCacheControl(options.versioned, context),
     })
   } catch (error) {
-    logger.error('Error downloading from cloud storage:', error)
+    logServeFailure('Error downloading from cloud storage:', error)
     throw error
   }
 }
@@ -348,7 +365,7 @@ async function handleCloudProxyPublic(
       cacheControl: PUBLIC_ASSET_CACHE_CONTROL,
     })
   } catch (error) {
-    logger.error('Error serving public cloud file:', error)
+    logServeFailure('Error serving public cloud file:', error)
     throw error
   }
 }
@@ -373,7 +390,7 @@ async function handleLocalFilePublic(filename: string): Promise<NextResponse> {
       cacheControl: PUBLIC_ASSET_CACHE_CONTROL,
     })
   } catch (error) {
-    logger.error('Error reading public local file:', error)
+    logServeFailure('Error reading public local file:', error)
     throw error
   }
 }

apps/sim/lib/uploads/core/errors.test.ts

@@ -0,0 +1,69 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
+
+describe('isObjectNotFoundError', () => {
+  it('matches the shapes each storage provider uses for a missing object', () => {
+    /** S3 HeadObject, exactly as the production payload arrived. */
+    expect(
+      isObjectNotFoundError({
+        name: 'NotFound',
+        $fault: 'client',
+        $metadata: { httpStatusCode: 404 },
+      })
+    ).toBe(true)
+    /** S3 GetObject. */
+    expect(isObjectNotFoundError({ name: 'NoSuchKey', $metadata: { httpStatusCode: 404 } })).toBe(
+      true
+    )
+    /** Azure Blob. */
+    expect(isObjectNotFoundError({ code: 'BlobNotFound', statusCode: 404 })).toBe(true)
+    /** GCS, which reports a numeric code. */
+    expect(isObjectNotFoundError({ code: 404 })).toBe(true)
+  })
+
+  it('reads the label from code when name carries the error class instead', () => {
+    /** Azure raises a `RestError`; the reason lives in `code`, not `name`. */
+    expect(isObjectNotFoundError({ name: 'RestError', code: 'BlobNotFound' })).toBe(true)
+    expect(isObjectNotFoundError({ name: 'Error', code: 'NoSuchKey' })).toBe(true)
+  })
+
+  it('does not read a missing bucket or container as an absent object', () => {
+    /**
+     * These answer 404 too. Reading them as absence would turn a total storage
+     * misconfiguration into silent fail-closed reads with nothing to alert on.
+     */
+    expect(
+      isObjectNotFoundError({ name: 'NoSuchBucket', $metadata: { httpStatusCode: 404 } })
+    ).toBe(false)
+    expect(
+      isObjectNotFoundError({ name: 'RestError', code: 'ContainerNotFound', statusCode: 404 })
+    ).toBe(false)
+  })
+
+  it('matches on status alone when the provider sends no label', () => {
+    expect(isObjectNotFoundError({ $metadata: { httpStatusCode: 404 } })).toBe(true)
+    expect(isObjectNotFoundError({ statusCode: 404 })).toBe(true)
+  })
+
+  it('does not swallow a genuine failure', () => {
+    expect(
+      isObjectNotFoundError({ name: 'AccessDenied', $metadata: { httpStatusCode: 403 } })
+    ).toBe(false)
+    expect(
+      isObjectNotFoundError({ name: 'InternalError', $metadata: { httpStatusCode: 500 } })
+    ).toBe(false)
+    expect(isObjectNotFoundError({ name: 'TimeoutError' })).toBe(false)
+    expect(isObjectNotFoundError({ code: 'ECONNRESET' })).toBe(false)
+    expect(isObjectNotFoundError({ code: 403 })).toBe(false)
+  })
+
+  it('tolerates values that are not error objects', () => {
+    expect(isObjectNotFoundError(null)).toBe(false)
+    expect(isObjectNotFoundError(undefined)).toBe(false)
+    expect(isObjectNotFoundError('NotFound')).toBe(false)
+    expect(isObjectNotFoundError(404)).toBe(false)
+  })
+})

apps/sim/lib/uploads/core/errors.ts

@@ -0,0 +1,47 @@
+const OBJECT_NOT_FOUND_LABELS = new Set(['NotFound', 'NoSuchKey', 'BlobNotFound'])
+
+/**
+ * A missing bucket or container is a misconfiguration, not an absent object, and
+ * it also answers 404. Without this it would read as "no metadata" and every file
+ * read would fail closed with no error to alert on.
+ */
+const CONTAINER_NOT_FOUND_LABELS = new Set(['NoSuchBucket', 'ContainerNotFound'])
+
+function readLabels(error: unknown): string[] | null {
+  if (!error || typeof error !== 'object') return null
+  const { name, code } = error as { name?: unknown; code?: unknown }
+  /**
+   * `name` and `code` are both consulted: Azure raises a `RestError` whose `name`
+   * carries the class and whose `code` carries the reason, while the AWS SDK puts
+   * the reason in `name`.
+   */
+  return [name, code].filter((value): value is string => typeof value === 'string')
+}
+
+/**
+ * True when a storage provider reports that an object does not exist.
+ *
+ * Call this only from code that has just performed an object-level operation, so a
+ * bare 404 can be attributed to that object. A bare 404 is otherwise ambiguous —
+ * GCS answers a missing object and a missing bucket identically (`code: 404`,
+ * `errors[].reason: 'notFound'`), separable only by a human-readable message — and
+ * every caller here is a provider client that knows exactly what it asked for.
+ *
+ * Absence is an expected outcome of a lookup, so callers turn it into an empty
+ * result rather than propagating it.
+ *
+ * A network failure, a permission denial, or a provider 5xx still propagates.
+ */
+export function isObjectNotFoundError(error: unknown): boolean {
+  const labels = readLabels(error)
+  if (!labels) return false
+  if (labels.some((label) => CONTAINER_NOT_FOUND_LABELS.has(label))) return false
+  if (labels.some((label) => OBJECT_NOT_FOUND_LABELS.has(label))) return true
+
+  const { code, statusCode, $metadata } = error as {
+    code?: unknown
+    statusCode?: unknown
+    $metadata?: { httpStatusCode?: unknown }
+  }
+  return code === 404 || statusCode === 404 || $metadata?.httpStatusCode === 404
+}

apps/sim/lib/uploads/core/storage-client.test.ts

@@ -0,0 +1,78 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockGetFileMetadataByKey, mockHeadS3Object } = vi.hoisted(() => ({
+  mockGetFileMetadataByKey: vi.fn(),
+  mockHeadS3Object: vi.fn(),
+}))
+
+vi.mock('@/lib/uploads/config', () => ({
+  USE_S3_STORAGE: true,
+  USE_BLOB_STORAGE: false,
+  USE_GCS_STORAGE: false,
+  S3_CONFIG: { bucket: 'bucket', region: 'region' },
+}))
+
+vi.mock('@/lib/uploads/providers/s3/client', () => ({
+  headS3Object: mockHeadS3Object,
+}))
+
+vi.mock('@/lib/uploads/server/metadata', () => ({
+  getFileMetadataByKey: mockGetFileMetadataByKey,
+}))
+
+import { getFileMetadata } from '@/lib/uploads/core/storage-client'
+
+describe('getFileMetadata', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockGetFileMetadataByKey.mockResolvedValue(null)
+  })
+
+  it('reports an absent object as no metadata rather than throwing', async () => {
+    /** The provider client owns not-found and reports absence as `null`. */
+    mockHeadS3Object.mockResolvedValue(null)
+
+    await expect(getFileMetadata('workspace/ws/superseded-key.md')).resolves.toEqual({})
+  })
+
+  it('still propagates a genuine storage failure', async () => {
+    mockHeadS3Object.mockRejectedValue(
+      Object.assign(new Error('AccessDenied'), {
+        name: 'AccessDenied',
+        $metadata: { httpStatusCode: 403 },
+      })
+    )
+
+    await expect(getFileMetadata('workspace/ws/key.md')).rejects.toThrow('AccessDenied')
+  })
+
+  it('returns provider metadata when the object exists', async () => {
+    mockHeadS3Object.mockResolvedValue({ size: 12, metadata: { workspaceid: 'ws-1' } })
+
+    await expect(getFileMetadata('workspace/ws/key.md')).resolves.toEqual({ workspaceid: 'ws-1' })
+  })
+
+  it('treats an object carrying no metadata as no metadata', async () => {
+    mockHeadS3Object.mockResolvedValue({ size: 12 })
+
+    await expect(getFileMetadata('workspace/ws/key.md')).resolves.toEqual({})
+  })
+
+  it('prefers the database record when one exists', async () => {
+    mockGetFileMetadataByKey.mockResolvedValue({
+      userId: 'user-1',
+      workspaceId: 'ws-1',
+      originalName: 'doc.md',
+      uploadedAt: new Date('2026-01-01T00:00:00Z'),
+      context: 'workspace',
+    })
+
+    const metadata = await getFileMetadata('workspace/ws/key.md')
+
+    expect(metadata.workspaceId).toBe('ws-1')
+    expect(mockHeadS3Object).not.toHaveBeenCalled()
+  })
+})