From 018317dfbaa1655e083affe465dd5f71d41626bc Mon Sep 17 00:00:00 2001 From: Anders Semb Hermansen Date: Wed, 11 Jun 2025 16:49:34 +0200 Subject: [PATCH] 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. --- packages/storage-azure/src/staticHandler.ts | 4 + pnpm-lock.yaml | 97 +++++++++++++++- test/package.json | 1 + test/storage-azure/int.spec.ts | 116 ++++++++++++++++++++ tsconfig.base.json | 1 + 5 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 test/storage-azure/int.spec.ts diff --git a/packages/storage-azure/src/staticHandler.ts b/packages/storage-azure/src/staticHandler.ts index 58ea821120..915c7de94d 100644 --- a/packages/storage-azure/src/staticHandler.ts +++ b/packages/storage-azure/src/staticHandler.ts @@ -2,6 +2,7 @@ import type { ContainerClient } from '@azure/storage-blob' import type { StaticHandler } from '@payloadcms/plugin-cloud-storage/types' import type { CollectionConfig } from 'payload' +import { RestError } from '@azure/storage-blob' import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities' import path from 'path' @@ -66,6 +67,9 @@ export const getHandler = ({ collection, getStorageClient }: Args): StaticHandle status: response.status, }) } catch (err: unknown) { + if (err instanceof RestError && err.statusCode === 404) { + return new Response(null, { status: 404, statusText: 'Not Found' }) + } req.payload.logger.error(err) return new Response('Internal Server Error', { status: 500 }) } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1c40cb1c71..6c3df41e2c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -128,7 +128,7 @@ importers: version: 10.1.4(@aws-sdk/credential-providers@3.687.0(@aws-sdk/client-sso-oidc@3.687.0(@aws-sdk/client-sts@3.687.0)))(socks@2.8.3) next: specifier: 15.3.2 - version: 15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) + version: 15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) open: specifier: ^10.1.0 version: 10.1.0 @@ -1669,6 +1669,9 @@ importers: '@aws-sdk/client-s3': specifier: ^3.614.0 version: 3.687.0 + '@azure/storage-blob': + specifier: ^12.11.0 + version: 12.25.0 '@date-fns/tz': specifier: 1.2.0 version: 1.2.0 @@ -1779,7 +1782,7 @@ importers: version: link:../packages/ui '@sentry/nextjs': specifier: ^8.33.1 - version: 8.37.1(@opentelemetry/core@1.27.0(@opentelemetry/api@1.9.0))(@opentelemetry/instrumentation@0.54.2(@opentelemetry/api@1.9.0))(@opentelemetry/sdk-trace-base@1.27.0(@opentelemetry/api@1.9.0))(next@15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4))(react@19.1.0)(webpack@5.96.1(@swc/core@1.11.29)) + version: 8.37.1(@opentelemetry/core@1.27.0(@opentelemetry/api@1.9.0))(@opentelemetry/instrumentation@0.54.2(@opentelemetry/api@1.9.0))(@opentelemetry/sdk-trace-base@1.27.0(@opentelemetry/api@1.9.0))(next@15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4))(react@19.1.0)(webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12)) '@sentry/react': specifier: ^7.77.0 version: 7.119.2(react@19.1.0) @@ -1842,7 +1845,7 @@ importers: version: 8.15.1(@aws-sdk/credential-providers@3.687.0(@aws-sdk/client-sso-oidc@3.687.0(@aws-sdk/client-sts@3.687.0)))(socks@2.8.3) next: specifier: 15.3.2 - version: 15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) + version: 15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) nodemailer: specifier: 6.9.16 version: 6.9.16 @@ -8137,6 +8140,7 @@ packages: libsql@0.4.7: resolution: {integrity: sha512-T9eIRCs6b0J1SHKYIvD8+KCJMcWZ900iZyxdnSCdqxN12Z1ijzT+jY5nrk72Jw4B0HGzms2NgpryArlJqvc3Lw==} + cpu: [x64, arm64, wasm32] os: [darwin, linux, win32] license-checker@25.0.1: @@ -14137,6 +14141,35 @@ snapshots: '@sentry/utils': 7.119.2 localforage: 1.10.0 + '@sentry/nextjs@8.37.1(@opentelemetry/core@1.27.0(@opentelemetry/api@1.9.0))(@opentelemetry/instrumentation@0.54.2(@opentelemetry/api@1.9.0))(@opentelemetry/sdk-trace-base@1.27.0(@opentelemetry/api@1.9.0))(next@15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4))(react@19.1.0)(webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12))': + dependencies: + '@opentelemetry/api': 1.9.0 + '@opentelemetry/instrumentation-http': 0.53.0(@opentelemetry/api@1.9.0) + '@opentelemetry/semantic-conventions': 1.27.0 + '@rollup/plugin-commonjs': 26.0.1(rollup@3.29.5) + '@sentry-internal/browser-utils': 8.37.1 + '@sentry/core': 8.37.1 + '@sentry/node': 8.37.1 + '@sentry/opentelemetry': 8.37.1(@opentelemetry/api@1.9.0)(@opentelemetry/core@1.27.0(@opentelemetry/api@1.9.0))(@opentelemetry/instrumentation@0.54.2(@opentelemetry/api@1.9.0))(@opentelemetry/sdk-trace-base@1.27.0(@opentelemetry/api@1.9.0))(@opentelemetry/semantic-conventions@1.27.0) + '@sentry/react': 8.37.1(react@19.1.0) + '@sentry/types': 8.37.1 + '@sentry/utils': 8.37.1 + '@sentry/vercel-edge': 8.37.1 + '@sentry/webpack-plugin': 2.22.6(webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12)) + chalk: 3.0.0 + next: 15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) + resolve: 1.22.8 + rollup: 3.29.5 + stacktrace-parser: 0.1.10 + transitivePeerDependencies: + - '@opentelemetry/core' + - '@opentelemetry/instrumentation' + - '@opentelemetry/sdk-trace-base' + - encoding + - react + - supports-color + - webpack + '@sentry/nextjs@8.37.1(@opentelemetry/core@1.27.0(@opentelemetry/api@1.9.0))(@opentelemetry/instrumentation@0.54.2(@opentelemetry/api@1.9.0))(@opentelemetry/sdk-trace-base@1.27.0(@opentelemetry/api@1.9.0))(next@15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4))(react@19.1.0)(webpack@5.96.1(@swc/core@1.11.29))': dependencies: '@opentelemetry/api': 1.9.0 @@ -14153,7 +14186,7 @@ snapshots: '@sentry/vercel-edge': 8.37.1 '@sentry/webpack-plugin': 2.22.6(webpack@5.96.1(@swc/core@1.11.29)) chalk: 3.0.0 - next: 15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) + next: 15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) resolve: 1.22.8 rollup: 3.29.5 stacktrace-parser: 0.1.10 @@ -14261,6 +14294,16 @@ snapshots: '@sentry/types': 8.37.1 '@sentry/utils': 8.37.1 + '@sentry/webpack-plugin@2.22.6(webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12))': + dependencies: + '@sentry/bundler-plugin-core': 2.22.6 + unplugin: 1.0.1 + uuid: 9.0.0 + webpack: 5.96.1(@swc/core@1.11.29)(esbuild@0.19.12) + transitivePeerDependencies: + - encoding + - supports-color + '@sentry/webpack-plugin@2.22.6(webpack@5.96.1(@swc/core@1.11.29))': dependencies: '@sentry/bundler-plugin-core': 2.22.6 @@ -18809,7 +18852,7 @@ snapshots: - '@babel/core' - babel-plugin-macros - next@15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4): + next@15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4): dependencies: '@next/env': 15.3.2 '@swc/counter': 0.1.3 @@ -20287,6 +20330,18 @@ snapshots: ansi-escapes: 4.3.2 supports-hyperlinks: 2.3.0 + terser-webpack-plugin@5.3.10(@swc/core@1.11.29)(esbuild@0.19.12)(webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12)): + dependencies: + '@jridgewell/trace-mapping': 0.3.25 + jest-worker: 27.5.1 + schema-utils: 3.3.0 + serialize-javascript: 6.0.2 + terser: 5.36.0 + webpack: 5.96.1(@swc/core@1.11.29)(esbuild@0.19.12) + optionalDependencies: + '@swc/core': 1.11.29 + esbuild: 0.19.12 + terser-webpack-plugin@5.3.10(@swc/core@1.11.29)(webpack@5.96.1(@swc/core@1.11.29)): dependencies: '@jridgewell/trace-mapping': 0.3.25 @@ -20578,7 +20633,7 @@ snapshots: '@uploadthing/shared': 7.1.1 effect: 3.10.3 optionalDependencies: - next: 15.3.2(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) + next: 15.3.2(@babel/core@7.27.3)(@opentelemetry/api@1.9.0)(@playwright/test@1.50.0)(babel-plugin-macros@3.1.0)(babel-plugin-react-compiler@19.1.0-rc.2)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(sass@1.77.4) uri-js@4.4.1: dependencies: @@ -20709,6 +20764,36 @@ snapshots: - esbuild - uglify-js + webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12): + dependencies: + '@types/eslint-scope': 3.7.7 + '@types/estree': 1.0.6 + '@webassemblyjs/ast': 1.14.1 + '@webassemblyjs/wasm-edit': 1.14.1 + '@webassemblyjs/wasm-parser': 1.14.1 + acorn: 8.14.0 + browserslist: 4.25.0 + chrome-trace-event: 1.0.4 + enhanced-resolve: 5.17.1 + es-module-lexer: 1.5.4 + eslint-scope: 5.1.1 + events: 3.3.0 + glob-to-regexp: 0.4.1 + graceful-fs: 4.2.11 + json-parse-even-better-errors: 2.3.1 + loader-runner: 4.3.0 + mime-types: 2.1.35 + neo-async: 2.6.2 + schema-utils: 3.3.0 + tapable: 2.2.1 + terser-webpack-plugin: 5.3.10(@swc/core@1.11.29)(esbuild@0.19.12)(webpack@5.96.1(@swc/core@1.11.29)(esbuild@0.19.12)) + watchpack: 2.4.2 + webpack-sources: 3.2.3 + transitivePeerDependencies: + - '@swc/core' + - esbuild + - uglify-js + whatwg-url@13.0.0: dependencies: tr46: 4.1.1 diff --git a/test/package.json b/test/package.json index 14b09b0428..03c527f73e 100644 --- a/test/package.json +++ b/test/package.json @@ -23,6 +23,7 @@ }, "devDependencies": { "@aws-sdk/client-s3": "^3.614.0", + "@azure/storage-blob": "^12.11.0", "@date-fns/tz": "1.2.0", "@next/env": "15.3.2", "@payloadcms/admin-bar": "workspace:*", diff --git a/test/storage-azure/int.spec.ts b/test/storage-azure/int.spec.ts new file mode 100644 index 0000000000..14cc8c9d13 --- /dev/null +++ b/test/storage-azure/int.spec.ts @@ -0,0 +1,116 @@ +import type { ContainerClient } from '@azure/storage-blob' +import type { CollectionSlug, Payload } from 'payload' + +import { BlobServiceClient } from '@azure/storage-blob' +import path from 'path' +import { fileURLToPath } from 'url' + +import type { NextRESTClient } from '../helpers/NextRESTClient.js' + +import { initPayloadInt } from '../helpers/initPayloadInt.js' +import { mediaSlug, mediaWithPrefixSlug, prefix } from './shared.js' + +const filename = fileURLToPath(import.meta.url) +const dirname = path.dirname(filename) + +let restClient: NextRESTClient +let payload: Payload + +describe('@payloadcms/storage-azure', () => { + let TEST_CONTAINER: string + let client: ContainerClient + + beforeAll(async () => { + ;({ payload, restClient } = await initPayloadInt(dirname)) + TEST_CONTAINER = process.env.AZURE_STORAGE_CONTAINER_NAME! + + const blobServiceClient = BlobServiceClient.fromConnectionString( + process.env.AZURE_STORAGE_CONNECTION_STRING!, + ) + client = blobServiceClient.getContainerClient(TEST_CONTAINER) + + await client.createIfNotExists() + await clearContainer() + }) + + afterAll(async () => { + await payload.destroy() + }) + + afterEach(async () => { + await clearContainer() + }) + + it('can upload', async () => { + const upload = await payload.create({ + collection: mediaSlug, + data: {}, + filePath: path.resolve(dirname, '../uploads/image.png'), + }) + + expect(upload.id).toBeTruthy() + await verifyUploads({ collectionSlug: mediaSlug, uploadId: upload.id }) + expect(upload.url).toEqual(`/api/${mediaSlug}/file/${String(upload.filename)}`) + }) + + it('can upload with prefix', async () => { + const upload = await payload.create({ + collection: mediaWithPrefixSlug, + data: {}, + filePath: path.resolve(dirname, '../uploads/image.png'), + }) + + expect(upload.id).toBeTruthy() + await verifyUploads({ + collectionSlug: mediaWithPrefixSlug, + uploadId: upload.id, + prefix, + }) + expect(upload.url).toEqual(`/api/${mediaWithPrefixSlug}/file/${String(upload.filename)}`) + }) + + it('returns 404 for non-existing file', async () => { + const response = await restClient.GET(`/${mediaSlug}/file/nonexistent.png`) + expect(response.status).toBe(404) + }) + + async function clearContainer() { + for await (const blob of client.listBlobsFlat()) { + await client.deleteBlob(blob.name) + } + } + + async function verifyUploads({ + collectionSlug, + uploadId, + prefix = '', + }: { + collectionSlug: CollectionSlug + prefix?: string + uploadId: number | string + }) { + const uploadData = (await payload.findByID({ + collection: collectionSlug, + id: uploadId, + })) as unknown as { filename: string; sizes: Record } + + const fileKeys = Object.keys(uploadData.sizes || {}).map((key) => { + const rawFilename = uploadData.sizes[key].filename + return prefix ? `${prefix}/${rawFilename}` : rawFilename + }) + + fileKeys.push(`${prefix ? `${prefix}/` : ''}${uploadData.filename}`) + + for (const key of fileKeys) { + const blobClient = client.getBlobClient(key) + try { + const props = await blobClient.getProperties() + expect(props).toBeDefined() + expect(props.contentLength).toBeGreaterThan(0) + } catch (error) { + console.error('Error verifying uploads:', key, error) + throw error + } + } + } +}) diff --git a/tsconfig.base.json b/tsconfig.base.json index 8d0bb793bc..39285646fb 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -78,6 +78,7 @@ "./packages/plugin-multi-tenant/src/translations/languages/*.ts" ], "@payloadcms/next": ["./packages/next/src/exports/*"], + "@payloadcms/storage-azure/client": ["./packages/storage-azure/src/exports/client.ts"], "@payloadcms/storage-s3/client": ["./packages/storage-s3/src/exports/client.ts"], "@payloadcms/storage-vercel-blob/client": [ "./packages/storage-vercel-blob/src/exports/client.ts"