Skip to content

Commit 018317d

Browse files
fix(storage-azure): return error status 404 when file is not found instead of 500 (#11734)
### What? The azure storage adapter returns a 500 internal server error when a file is not found. It's expected that it will return 404 when a file is not found. ### Why? There is no checking if the blockBlobClient exists before it's used, so it throws a RestError when used and the blob does not exist. ### How? Check if exception thrown is of type RestError and have a 404 error from the Azure API and return a 404 in that case. An alternative way would be to call the exists() method on the blockBlobClient, but that will be one more API call for blobs that does exist. So I chose to check the exception instead. Also added integration tests for azure storage in the same manner as s3, as it was missing for azure storage.
1 parent 37afbe6 commit 018317d

File tree

5 files changed

+213
-6
lines changed

5 files changed

+213
-6
lines changed

packages/storage-azure/src/staticHandler.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ContainerClient } from '@azure/storage-blob'
22
import type { StaticHandler } from '@payloadcms/plugin-cloud-storage/types'
33
import type { CollectionConfig } from 'payload'
44

5+
import { RestError } from '@azure/storage-blob'
56
import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities'
67
import path from 'path'
78

@@ -66,6 +67,9 @@ export const getHandler = ({ collection, getStorageClient }: Args): StaticHandle
6667
status: response.status,
6768
})
6869
} catch (err: unknown) {
70+
if (err instanceof RestError && err.statusCode === 404) {
71+
return new Response(null, { status: 404, statusText: 'Not Found' })
72+
}
6973
req.payload.logger.error(err)
7074
return new Response('Internal Server Error', { status: 500 })
7175
}

pnpm-lock.yaml

Lines changed: 91 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
},
2424
"devDependencies": {
2525
"@aws-sdk/client-s3": "^3.614.0",
26+
"@azure/storage-blob": "^12.11.0",
2627
"@date-fns/tz": "1.2.0",
2728
"@next/env": "15.3.2",
2829
"@payloadcms/admin-bar": "workspace:*",

test/storage-azure/int.spec.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import type { ContainerClient } from '@azure/storage-blob'
2+
import type { CollectionSlug, Payload } from 'payload'
3+
4+
import { BlobServiceClient } from '@azure/storage-blob'
5+
import path from 'path'
6+
import { fileURLToPath } from 'url'
7+
8+
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
9+
10+
import { initPayloadInt } from '../helpers/initPayloadInt.js'
11+
import { mediaSlug, mediaWithPrefixSlug, prefix } from './shared.js'
12+
13+
const filename = fileURLToPath(import.meta.url)
14+
const dirname = path.dirname(filename)
15+
16+
let restClient: NextRESTClient
17+
let payload: Payload
18+
19+
describe('@payloadcms/storage-azure', () => {
20+
let TEST_CONTAINER: string
21+
let client: ContainerClient
22+
23+
beforeAll(async () => {
24+
;({ payload, restClient } = await initPayloadInt(dirname))
25+
TEST_CONTAINER = process.env.AZURE_STORAGE_CONTAINER_NAME!
26+
27+
const blobServiceClient = BlobServiceClient.fromConnectionString(
28+
process.env.AZURE_STORAGE_CONNECTION_STRING!,
29+
)
30+
client = blobServiceClient.getContainerClient(TEST_CONTAINER)
31+
32+
await client.createIfNotExists()
33+
await clearContainer()
34+
})
35+
36+
afterAll(async () => {
37+
await payload.destroy()
38+
})
39+
40+
afterEach(async () => {
41+
await clearContainer()
42+
})
43+
44+
it('can upload', async () => {
45+
const upload = await payload.create({
46+
collection: mediaSlug,
47+
data: {},
48+
filePath: path.resolve(dirname, '../uploads/image.png'),
49+
})
50+
51+
expect(upload.id).toBeTruthy()
52+
await verifyUploads({ collectionSlug: mediaSlug, uploadId: upload.id })
53+
expect(upload.url).toEqual(`/api/${mediaSlug}/file/${String(upload.filename)}`)
54+
})
55+
56+
it('can upload with prefix', async () => {
57+
const upload = await payload.create({
58+
collection: mediaWithPrefixSlug,
59+
data: {},
60+
filePath: path.resolve(dirname, '../uploads/image.png'),
61+
})
62+
63+
expect(upload.id).toBeTruthy()
64+
await verifyUploads({
65+
collectionSlug: mediaWithPrefixSlug,
66+
uploadId: upload.id,
67+
prefix,
68+
})
69+
expect(upload.url).toEqual(`/api/${mediaWithPrefixSlug}/file/${String(upload.filename)}`)
70+
})
71+
72+
it('returns 404 for non-existing file', async () => {
73+
const response = await restClient.GET(`/${mediaSlug}/file/nonexistent.png`)
74+
expect(response.status).toBe(404)
75+
})
76+
77+
async function clearContainer() {
78+
for await (const blob of client.listBlobsFlat()) {
79+
await client.deleteBlob(blob.name)
80+
}
81+
}
82+
83+
async function verifyUploads({
84+
collectionSlug,
85+
uploadId,
86+
prefix = '',
87+
}: {
88+
collectionSlug: CollectionSlug
89+
prefix?: string
90+
uploadId: number | string
91+
}) {
92+
const uploadData = (await payload.findByID({
93+
collection: collectionSlug,
94+
id: uploadId,
95+
})) as unknown as { filename: string; sizes: Record<string, { filename: string }> }
96+
97+
const fileKeys = Object.keys(uploadData.sizes || {}).map((key) => {
98+
const rawFilename = uploadData.sizes[key].filename
99+
return prefix ? `${prefix}/${rawFilename}` : rawFilename
100+
})
101+
102+
fileKeys.push(`${prefix ? `${prefix}/` : ''}${uploadData.filename}`)
103+
104+
for (const key of fileKeys) {
105+
const blobClient = client.getBlobClient(key)
106+
try {
107+
const props = await blobClient.getProperties()
108+
expect(props).toBeDefined()
109+
expect(props.contentLength).toBeGreaterThan(0)
110+
} catch (error) {
111+
console.error('Error verifying uploads:', key, error)
112+
throw error
113+
}
114+
}
115+
}
116+
})

tsconfig.base.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
"./packages/plugin-multi-tenant/src/translations/languages/*.ts"
7979
],
8080
"@payloadcms/next": ["./packages/next/src/exports/*"],
81+
"@payloadcms/storage-azure/client": ["./packages/storage-azure/src/exports/client.ts"],
8182
"@payloadcms/storage-s3/client": ["./packages/storage-s3/src/exports/client.ts"],
8283
"@payloadcms/storage-vercel-blob/client": [
8384
"./packages/storage-vercel-blob/src/exports/client.ts"

0 commit comments

Comments
 (0)