fix(uploads): require an object-level label before treating a lookup as absent

GCS answers a missing object and a missing bucket identically, so a bare 404
cannot be attributed to the object by a dispatcher that does not know what was
requested. getFileMetadata now takes the labelled check and leaves an unlabelled
404 propagating as before; the provider clients keep the lenient form, which is
what each already used.

Author: waleedlatif1

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

@@ -2,7 +2,7 @@
  * @vitest-environment node
  */
 import { describe, expect, it } from 'vitest'
-import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
+import { hasObjectNotFoundLabel, isObjectNotFoundError } from '@/lib/uploads/core/errors'
 
 describe('isObjectNotFoundError', () => {
   it('matches the shapes each storage provider uses for a missing object', () => {
@@ -67,3 +67,23 @@ describe('isObjectNotFoundError', () => {
     expect(isObjectNotFoundError(404)).toBe(false)
   })
 })
+
+describe('hasObjectNotFoundLabel', () => {
+  it('accepts only a provider that names the object as the missing resource', () => {
+    expect(hasObjectNotFoundLabel({ name: 'NotFound' })).toBe(true)
+    expect(hasObjectNotFoundLabel({ name: 'NoSuchKey' })).toBe(true)
+    expect(hasObjectNotFoundLabel({ name: 'RestError', code: 'BlobNotFound' })).toBe(true)
+  })
+
+  it('rejects an unlabelled 404, which cannot be attributed to the object', () => {
+    /** GCS answers a missing object and a missing bucket identically. */
+    expect(hasObjectNotFoundLabel({ code: 404 })).toBe(false)
+    expect(hasObjectNotFoundLabel({ statusCode: 404 })).toBe(false)
+    expect(hasObjectNotFoundLabel({ $metadata: { httpStatusCode: 404 } })).toBe(false)
+  })
+
+  it('rejects a missing bucket or container', () => {
+    expect(hasObjectNotFoundLabel({ name: 'NoSuchBucket' })).toBe(false)
+    expect(hasObjectNotFoundLabel({ name: 'RestError', code: 'ContainerNotFound' })).toBe(false)
+  })
+})

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

@@ -7,37 +7,55 @@ const OBJECT_NOT_FOUND_LABELS = new Set(['NotFound', 'NoSuchKey', 'BlobNotFound'
  */
 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 provider names the missing resource as the object itself.
+ *
+ * The strict form. Use it wherever the caller cannot vouch for what was requested,
+ * because an unlabelled 404 is ambiguous: GCS answers a missing object and a
+ * missing bucket identically (`code: 404`, `errors[].reason: 'notFound'`), so only
+ * the human-readable message separates them. Treating that as absence would let a
+ * bucket misconfiguration read as "no metadata" and fail every read closed with
+ * nothing left to alert on, so it stays an error here.
+ */
+export function hasObjectNotFoundLabel(error: unknown): boolean {
+  const labels = readLabels(error)
+  if (!labels) return false
+  if (labels.some((label) => CONTAINER_NOT_FOUND_LABELS.has(label))) return false
+  return labels.some((label) => OBJECT_NOT_FOUND_LABELS.has(label))
+}
+
 /**
- * True when a storage provider reports that an object simply does not exist.
+ * True when a storage provider reports that an object does not exist.
  *
- * Absence is an expected outcome of a lookup, not a failure, so callers turn this
- * into an empty result rather than propagating it. Every provider spells it
- * differently — S3 throws `NotFound` (HeadObject) or `NoSuchKey` (GetObject),
- * Azure Blob throws `BlobNotFound`, and GCS throws a numeric `code: 404` — and the
- * status may arrive as `$metadata.httpStatusCode`, `statusCode`, or `code`.
+ * The lenient form, for a caller that has just performed an object-level operation
+ * and can therefore attribute a bare 404 to that object — which is what every
+ * provider client here does, and how each spelled this check before it was shared.
+ * It additionally accepts a bare 404 (`code`, `statusCode`, or
+ * `$metadata.httpStatusCode`), which GCS relies on since it carries no label.
  *
- * Only genuine absence matches. A network failure, a permission denial, or a
- * provider 5xx still propagates so it surfaces as the error it is.
+ * A network failure, a permission denial, or a provider 5xx still propagates.
  */
 export function isObjectNotFoundError(error: unknown): boolean {
-  if (!error || typeof error !== 'object') return false
+  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 { name, code, statusCode, $metadata } = error as {
-    name?: unknown
+  const { code, statusCode, $metadata } = error as {
     code?: unknown
     statusCode?: unknown
     $metadata?: { httpStatusCode?: 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`.
-   */
-  const labels = [name, code].filter((value): value is string => typeof value === 'string')
-
-  if (labels.some((label) => CONTAINER_NOT_FOUND_LABELS.has(label))) return false
-  if (labels.some((label) => OBJECT_NOT_FOUND_LABELS.has(label))) return true
-
   return code === 404 || statusCode === 404 || $metadata?.httpStatusCode === 404
 }

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

@@ -1,5 +1,5 @@
 import { USE_BLOB_STORAGE, USE_GCS_STORAGE, USE_S3_STORAGE } from '@/lib/uploads/config'
-import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
+import { hasObjectNotFoundLabel } from '@/lib/uploads/core/errors'
 import type { StorageConfig } from '@/lib/uploads/shared/types'
 
 export type { StorageConfig } from '@/lib/uploads/shared/types'
@@ -40,8 +40,12 @@ export async function getFileMetadata(
      * previous key finds nothing. Report it the way this function already reports
      * "nothing known about this key" rather than as a failure, so callers fall
      * through to their own not-found handling instead of an error path.
+     *
+     * Deliberately the labelled check: this dispatches across every provider and so
+     * cannot attribute a bare 404 to the object rather than its bucket. An
+     * unlabelled 404 keeps propagating, exactly as it did before.
      */
-    if (isObjectNotFoundError(error)) return {}
+    if (hasObjectNotFoundLabel(error)) return {}
     throw error
   }
 }