diff options
author | 2024-01-30 18:15:26 +0100 | |
---|---|---|
committer | 2024-01-30 22:45:26 +0530 | |
commit | e9027f194b939ac5a4d795ee1a2c24e4a6fbefc0 (patch) | |
tree | 262e903114aa0b66a9be76e71c4df2268bd701a4 | |
parent | 3a50cbf6ed229869f67e1246fb76bbe91b0f81cd (diff) | |
download | astro-e9027f194b939ac5a4d795ee1a2c24e4a6fbefc0.tar.gz astro-e9027f194b939ac5a4d795ee1a2c24e4a6fbefc0.tar.zst astro-e9027f194b939ac5a4d795ee1a2c24e4a6fbefc0.zip |
Fix endpoint response with immutable header (#9876)
* add test
* added runtime endpoint test for `new Response`
* added `try..catch` for reroute directive handling
Fixes #9871
* added changeset
* replaced `try..catch` with HTTP status code check
based on the suggestion of @lilnasy
* updated changeset description
* added more tests for the endpoint reroute header
* fixed grammar in `renderEndpoint` comment
* updated endpoint tests to check for the reroute directive header in lower-case
* updated changeset description
-rw-r--r-- | .changeset/silver-peaches-pump.md | 5 | ||||
-rw-r--r-- | packages/astro/src/runtime/server/endpoint.ts | 8 | ||||
-rw-r--r-- | packages/astro/test/units/routing/endpoints.test.js | 91 |
3 files changed, 103 insertions, 1 deletions
diff --git a/.changeset/silver-peaches-pump.md b/.changeset/silver-peaches-pump.md new file mode 100644 index 000000000..67bdfa4fd --- /dev/null +++ b/.changeset/silver-peaches-pump.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes an issue where using `Response.redirect` in an endpoint led to an error. diff --git a/packages/astro/src/runtime/server/endpoint.ts b/packages/astro/src/runtime/server/endpoint.ts index afafb21c7..49f00224e 100644 --- a/packages/astro/src/runtime/server/endpoint.ts +++ b/packages/astro/src/runtime/server/endpoint.ts @@ -42,6 +42,12 @@ export async function renderEndpoint( const response = await handler.call(mod, context); // Endpoints explicitly returning 404 or 500 response status should // NOT be subject to rerouting to 404.astro or 500.astro. - response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no'); + if (response.status === 404 || response.status === 500) { + // Only `Response.redirect` headers are immutable, therefore a `try..catch` is not necessary. + // Note: `Response.redirect` can only be called with HTTP status codes: 301, 302, 303, 307, 308. + // Source: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#parameters + response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no'); + } + return response; } diff --git a/packages/astro/test/units/routing/endpoints.test.js b/packages/astro/test/units/routing/endpoints.test.js new file mode 100644 index 000000000..fde7a81da --- /dev/null +++ b/packages/astro/test/units/routing/endpoints.test.js @@ -0,0 +1,91 @@ +import { + createBasicSettings, + createFs, + createRequestAndResponse, + defaultLogger, +} from '../test-utils.js'; +import { fileURLToPath } from 'node:url'; +import { expect } from 'chai'; +import { createContainer } from '../../../dist/core/dev/container.js'; +import testAdapter from '../../test-adapter.js'; + +const root = new URL('../../fixtures/api-routes/', import.meta.url); +const fileSystem = { + '/src/pages/response-redirect.ts': `export const GET = ({ url }) => Response.redirect("https://example.com/destination", 307)`, + '/src/pages/response.ts': `export const GET = ({ url }) => new Response(null, { headers: { Location: "https://example.com/destination" }, status: 307 })`, + '/src/pages/not-found.ts': `export const GET = ({ url }) => new Response('empty', { headers: { "Content-Type": "text/plain" }, status: 404 })`, + '/src/pages/internal-error.ts': `export const GET = ({ url }) => new Response('something went wrong', { headers: { "Content-Type": "text/plain" }, status: 500 })`, +}; + +describe('endpoints', () => { + let container; + let settings; + + before(async () => { + const fs = createFs(fileSystem, root); + settings = await createBasicSettings({ + root: fileURLToPath(root), + output: 'server', + adapter: testAdapter(), + }); + container = await createContainer({ + fs, + settings, + logger: defaultLogger, + }); + }); + + after(async () => { + await container.close(); + }); + + it('should return a redirect response with location header', async () => { + const { req, res, done } = createRequestAndResponse({ + method: 'GET', + url: '/response-redirect', + }); + container.handle(req, res); + await done; + const headers = res.getHeaders(); + expect(headers).to.deep.include({ location: 'https://example.com/destination' }); + expect(headers).not.to.deep.include({ 'x-astro-reroute': 'no' }); + expect(res.statusCode).to.equal(307); + }); + + it('should return a response with location header', async () => { + const { req, res, done } = createRequestAndResponse({ + method: 'GET', + url: '/response', + }); + container.handle(req, res); + await done; + const headers = res.getHeaders(); + expect(headers).to.deep.include({ location: 'https://example.com/destination' }); + expect(headers).not.to.deep.include({ 'x-astro-reroute': 'no' }); + expect(res.statusCode).to.equal(307); + }); + + it('should append reroute header for HTTP status 404', async () => { + const { req, res, done } = createRequestAndResponse({ + method: 'GET', + url: '/not-found', + }); + container.handle(req, res); + await done; + const headers = res.getHeaders(); + expect(headers).to.deep.include({ 'x-astro-reroute': 'no' }); + expect(res.statusCode).to.equal(404); + }); + + it('should append reroute header for HTTP status 500', async () => { + const { req, res, done } = createRequestAndResponse({ + method: 'GET', + url: '/internal-error', + }); + container.handle(req, res); + await done; + const headers = res.getHeaders(); + expect(headers).to.deep.include({ 'x-astro-reroute': 'no' }); + expect(res.statusCode).to.equal(500); + }); +}); |