Skip to content

Commit

Permalink
fix(dev): dont reroute endpoint responses (#9830)
Browse files Browse the repository at this point in the history
* fix(dev): dont reroute endpoint responses

* factor out header name as a const

* add test case

* add changeset
  • Loading branch information
lilnasy authored Jan 25, 2024
1 parent e5276f0 commit f3d2213
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/rotten-masks-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes an issue where 404 responses from endpoints were replaced with contents of 404.astro in dev mode.
7 changes: 4 additions & 3 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
SSRManifest,
} from '../../@types/astro.js';
import { createI18nMiddleware, i18nPipelineHook } from '../../i18n/middleware.js';
import { REROUTE_DIRECTIVE_HEADER } from '../../runtime/server/consts.js';
import type { SinglePageBuiltModule } from '../build/types.js';
import { getSetCookiesFromResponse } from '../cookies/index.js';
import { consoleLogDestination } from '../logger/console.js';
Expand Down Expand Up @@ -278,16 +279,16 @@ export class App {

if (
REROUTABLE_STATUS_CODES.has(response.status) &&
response.headers.get('X-Astro-Reroute') !== 'no'
response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no'
) {
return this.#renderError(request, {
response,
status: response.status as 404 | 500,
});
}

if (response.headers.has('X-Astro-Reroute')) {
response.headers.delete('X-Astro-Reroute');
if (response.headers.has(REROUTE_DIRECTIVE_HEADER)) {
response.headers.delete(REROUTE_DIRECTIVE_HEADER);
}

if (addCookieHeader) {
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/runtime/server/consts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const REROUTE_DIRECTIVE_HEADER = 'X-Astro-Reroute';
3 changes: 2 additions & 1 deletion packages/astro/src/runtime/server/endpoint.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { bold } from 'kleur/colors';
import { REROUTE_DIRECTIVE_HEADER } from './consts.js';
import type { APIContext, EndpointHandler } from '../../@types/astro.js';
import type { Logger } from '../../core/logger/core.js';

Expand Down Expand Up @@ -41,6 +42,6 @@ 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('X-Astro-Reroute', 'no');
response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
return response;
}
3 changes: 2 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { preload } from './index.js';
import { getComponentMetadata } from './metadata.js';
import { handle404Response, writeSSRResult, writeWebResponse } from './response.js';
import { getScriptsForURL } from './scripts.js';
import { REROUTE_DIRECTIVE_HEADER } from '../runtime/server/consts.js';

const clientLocalsSymbol = Symbol.for('astro.locals');

Expand Down Expand Up @@ -342,7 +343,7 @@ export async function handleRoute({
})
);
}
if (response.status === 404 && has404Route(manifestData)) {
if (response.status === 404 && has404Route(manifestData) && response.headers.get(REROUTE_DIRECTIVE_HEADER) !== "no") {
const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline);
if (options && fourOhFourRoute?.route !== options.route)
return handleRoute({
Expand Down
8 changes: 8 additions & 0 deletions packages/astro/test/dev-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ describe('Development Routing', () => {
expect(body.title).to.equal('data [slug]');
});

it('error responses are served untouched', async () => {
const response = await fixture.fetch('/not-ok');
expect(response.status).to.equal(404);
expect(response.headers.get("Content-Type")).to.equal("text/plain;charset=UTF-8");
const body = await response.text();
expect(body).to.equal("Text from pages/not-ok.ts");
})

it('correct MIME type when loading /home.json (static route)', async () => {
const response = await fixture.fetch('/home.json');
expect(response.headers.get('content-type')).to.match(/application\/json/);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>Text from pages/404.astro</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export async function GET() {
return new Response("Text from pages/not-ok.ts", {
status: 404,
});
}

0 comments on commit f3d2213

Please sign in to comment.