Skip to content

Commit e93bf2e

Browse files
committed
fix(uploads): treat a missing storage object as absent metadata, not a failure
A workspace file is rewritten under a new key on every content update and the superseded object is deleted, so any reader holding the previous key finds nothing. getFileMetadata's provider lookups let that not-found propagate, so authorization's catch-all logged it at ERROR and never reached the branch already written for it. Return the function's established empty value instead, and collapse the three divergent per-provider not-found predicates onto one.
1 parent 1305e9d commit e93bf2e

7 files changed

Lines changed: 190 additions & 11 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
6+
7+
describe('isObjectNotFoundError', () => {
8+
it('matches the shapes each storage provider uses for a missing object', () => {
9+
/** S3 HeadObject, exactly as the production payload arrived. */
10+
expect(
11+
isObjectNotFoundError({
12+
name: 'NotFound',
13+
$fault: 'client',
14+
$metadata: { httpStatusCode: 404 },
15+
})
16+
).toBe(true)
17+
/** S3 GetObject. */
18+
expect(isObjectNotFoundError({ name: 'NoSuchKey', $metadata: { httpStatusCode: 404 } })).toBe(
19+
true
20+
)
21+
/** Azure Blob. */
22+
expect(isObjectNotFoundError({ code: 'BlobNotFound', statusCode: 404 })).toBe(true)
23+
/** GCS, which reports a numeric code. */
24+
expect(isObjectNotFoundError({ code: 404 })).toBe(true)
25+
})
26+
27+
it('matches on status alone when the provider sends no label', () => {
28+
expect(isObjectNotFoundError({ $metadata: { httpStatusCode: 404 } })).toBe(true)
29+
expect(isObjectNotFoundError({ statusCode: 404 })).toBe(true)
30+
})
31+
32+
it('does not swallow a genuine failure', () => {
33+
expect(
34+
isObjectNotFoundError({ name: 'AccessDenied', $metadata: { httpStatusCode: 403 } })
35+
).toBe(false)
36+
expect(
37+
isObjectNotFoundError({ name: 'InternalError', $metadata: { httpStatusCode: 500 } })
38+
).toBe(false)
39+
expect(isObjectNotFoundError({ name: 'TimeoutError' })).toBe(false)
40+
expect(isObjectNotFoundError({ code: 'ECONNRESET' })).toBe(false)
41+
expect(isObjectNotFoundError({ code: 403 })).toBe(false)
42+
})
43+
44+
it('tolerates values that are not error objects', () => {
45+
expect(isObjectNotFoundError(null)).toBe(false)
46+
expect(isObjectNotFoundError(undefined)).toBe(false)
47+
expect(isObjectNotFoundError('NotFound')).toBe(false)
48+
expect(isObjectNotFoundError(404)).toBe(false)
49+
})
50+
})
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* True when a storage provider reports that an object simply does not exist.
3+
*
4+
* Absence is an expected outcome of a lookup, not a failure, so callers turn this
5+
* into an empty result rather than propagating it. Every provider spells it
6+
* differently — S3 throws `NotFound` (HeadObject) or `NoSuchKey` (GetObject),
7+
* Azure Blob throws `BlobNotFound`, and GCS throws a numeric `code: 404` — and the
8+
* status may arrive as `$metadata.httpStatusCode`, `statusCode`, or `code`.
9+
*
10+
* Only genuine absence matches. A network failure, a permission denial, or a
11+
* provider 5xx still propagates so it surfaces as the error it is.
12+
*/
13+
export function isObjectNotFoundError(error: unknown): boolean {
14+
if (!error || typeof error !== 'object') return false
15+
16+
const candidate = error as {
17+
name?: unknown
18+
code?: unknown
19+
statusCode?: unknown
20+
$metadata?: { httpStatusCode?: unknown }
21+
}
22+
23+
const label = typeof candidate.name === 'string' ? candidate.name : candidate.code
24+
if (label === 'NotFound' || label === 'NoSuchKey' || label === 'BlobNotFound') return true
25+
26+
return (
27+
candidate.code === 404 ||
28+
candidate.statusCode === 404 ||
29+
candidate.$metadata?.httpStatusCode === 404
30+
)
31+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockGetFileMetadataByKey, mockSend, mockHeadObjectCommand } = vi.hoisted(() => ({
7+
mockGetFileMetadataByKey: vi.fn(),
8+
mockSend: vi.fn(),
9+
mockHeadObjectCommand: vi.fn().mockImplementation(class {}),
10+
}))
11+
12+
vi.mock('@aws-sdk/client-s3', () => ({
13+
HeadObjectCommand: mockHeadObjectCommand,
14+
}))
15+
16+
vi.mock('@/lib/uploads/config', () => ({
17+
USE_S3_STORAGE: true,
18+
USE_BLOB_STORAGE: false,
19+
USE_GCS_STORAGE: false,
20+
S3_CONFIG: { bucket: 'bucket', region: 'region' },
21+
}))
22+
23+
vi.mock('@/lib/uploads/providers/s3/client', () => ({
24+
getS3Client: () => ({ send: mockSend }),
25+
}))
26+
27+
vi.mock('@/lib/uploads/server/metadata', () => ({
28+
getFileMetadataByKey: mockGetFileMetadataByKey,
29+
}))
30+
31+
import { getFileMetadata } from '@/lib/uploads/core/storage-client'
32+
33+
/** The exact error the AWS SDK raises from HeadObject for an absent object. */
34+
const notFound = Object.assign(new Error('NotFound'), {
35+
name: 'NotFound',
36+
$fault: 'client',
37+
$metadata: { httpStatusCode: 404 },
38+
})
39+
40+
describe('getFileMetadata', () => {
41+
beforeEach(() => {
42+
vi.clearAllMocks()
43+
mockGetFileMetadataByKey.mockResolvedValue(null)
44+
})
45+
46+
it('reports an absent object as no metadata rather than throwing', async () => {
47+
mockSend.mockRejectedValue(notFound)
48+
49+
await expect(getFileMetadata('workspace/ws/superseded-key.md')).resolves.toEqual({})
50+
})
51+
52+
it('still propagates a genuine storage failure', async () => {
53+
const denied = Object.assign(new Error('AccessDenied'), {
54+
name: 'AccessDenied',
55+
$metadata: { httpStatusCode: 403 },
56+
})
57+
mockSend.mockRejectedValue(denied)
58+
59+
await expect(getFileMetadata('workspace/ws/key.md')).rejects.toThrow('AccessDenied')
60+
})
61+
62+
it('returns provider metadata when the object exists', async () => {
63+
mockSend.mockResolvedValue({ Metadata: { workspaceid: 'ws-1' } })
64+
65+
await expect(getFileMetadata('workspace/ws/key.md')).resolves.toEqual({ workspaceid: 'ws-1' })
66+
})
67+
68+
it('prefers the database record when one exists', async () => {
69+
mockGetFileMetadataByKey.mockResolvedValue({
70+
userId: 'user-1',
71+
workspaceId: 'ws-1',
72+
originalName: 'doc.md',
73+
uploadedAt: new Date('2026-01-01T00:00:00Z'),
74+
context: 'workspace',
75+
})
76+
77+
const metadata = await getFileMetadata('workspace/ws/key.md')
78+
79+
expect(metadata.workspaceId).toBe('ws-1')
80+
expect(mockSend).not.toHaveBeenCalled()
81+
})
82+
})

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { USE_BLOB_STORAGE, USE_GCS_STORAGE, USE_S3_STORAGE } from '@/lib/uploads/config'
2+
import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
23
import type { StorageConfig } from '@/lib/uploads/shared/types'
34

45
export type { StorageConfig } from '@/lib/uploads/shared/types'
@@ -29,6 +30,25 @@ export function getServePathPrefix(): string {
2930
export async function getFileMetadata(
3031
key: string,
3132
customConfig?: StorageConfig
33+
): Promise<Record<string, string>> {
34+
try {
35+
return await readProviderMetadata(key, customConfig)
36+
} catch (error) {
37+
/**
38+
* A key that no longer resolves is an ordinary outcome — a workspace file is
39+
* rewritten under a new key on every content update, so any reader holding the
40+
* previous key finds nothing. Report it the way this function already reports
41+
* "nothing known about this key" rather than as a failure, so callers fall
42+
* through to their own not-found handling instead of an error path.
43+
*/
44+
if (isObjectNotFoundError(error)) return {}
45+
throw error
46+
}
47+
}
48+
49+
async function readProviderMetadata(
50+
key: string,
51+
customConfig?: StorageConfig
3252
): Promise<Record<string, string>> {
3353
const { getFileMetadataByKey } = await import('../server/metadata')
3454
const metadataRecord = await getFileMetadataByKey(key)

apps/sim/lib/uploads/providers/blob/client.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {
2020
} from '@/lib/uploads/shared/types'
2121
import { sanitizeStorageMetadata } from '@/lib/uploads/utils/file-utils'
2222
import { sanitizeFileName } from '@/executor/constants'
23+
import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
2324

2425
const logger = createLogger('BlobClient')
2526
const MULTIPART_UPLOAD_ID_METADATA_KEY = 'sim_upload_id'
@@ -446,9 +447,7 @@ export async function headBlobObject(
446447
...(properties.metadata ? { metadata: properties.metadata } : {}),
447448
}
448449
} catch (err) {
449-
const status = (err as { statusCode?: number }).statusCode
450-
const code = (err as { code?: string }).code
451-
if (status === 404 || code === 'BlobNotFound') {
450+
if (isObjectNotFoundError(err)) {
452451
return null
453452
}
454453
throw err
@@ -833,9 +832,7 @@ export async function abortMultipartUpload(
833832
await blockBlobClient.deleteIfExists()
834833
}
835834
} catch (error) {
836-
const status = (error as { statusCode?: number }).statusCode
837-
const code = (error as { code?: string }).code
838-
if (status !== 404 && code !== 'BlobNotFound') {
835+
if (!isObjectNotFoundError(error)) {
839836
logger.warn('Error cleaning up multipart upload:', error)
840837
}
841838
}

apps/sim/lib/uploads/providers/gcs/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
sanitizeStorageMetadata,
2525
} from '@/lib/uploads/utils/file-utils'
2626
import { sanitizeFileName } from '@/executor/constants'
27+
import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
2728

2829
const logger = createLogger('GcsClient')
2930

@@ -404,7 +405,7 @@ async function getGcsMultipartCompletionId(
404405
const metadata = await getGcsObjectMetadata(key, customConfig)
405406
return metadata[GCS_MULTIPART_UPLOAD_ID_METADATA_KEY] ?? null
406407
} catch (error) {
407-
if ((error as { code?: number } | null)?.code === 404) return null
408+
if (isObjectNotFoundError(error)) return null
408409
throw error
409410
}
410411
}

apps/sim/lib/uploads/providers/s3/client.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
sanitizeStorageMetadata,
3737
} from '@/lib/uploads/utils/file-utils'
3838
import { sanitizeFileName } from '@/executor/constants'
39+
import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
3940

4041
let _s3Client: S3Client | null = null
4142

@@ -260,10 +261,7 @@ export async function headS3Object(
260261
...(response.Metadata ? { metadata: response.Metadata } : {}),
261262
}
262263
} catch (error) {
263-
const code = (error as { name?: string; $metadata?: { httpStatusCode?: number } } | null)?.name
264-
const status = (error as { $metadata?: { httpStatusCode?: number } } | null)?.$metadata
265-
?.httpStatusCode
266-
if (code === 'NotFound' || code === 'NoSuchKey' || status === 404) {
264+
if (isObjectNotFoundError(error)) {
267265
return null
268266
}
269267
throw error

0 commit comments

Comments
 (0)