Rewriting doesn't use HTTP status code from rewritten response

[vchirikov]

Astro Info

4.11.0

Describe the Bug

If we rewrite from non-existed page we always get 404 status even if middleware rewrite to the valid page.

Looks like the response object from rewriting isn't used. This is the repro:

we should get http status 200 (because we rewrite to the /about page), but we see 404.

p.s. The issue was originally described here, this is separate issue for the bug with repro.
p.s. looks similar to the cookie issue (fixed by @ascorbic )

What's the expected result?

rewriting should use status code from rewritten page pipeline.

Link to Minimal Reproducible Example

https://github.com/vchirikov/astro_issue_02

[ematipico]

@vchirikov do you experience the same issue with output: "server" (SSR)?

[vchirikov]

Yes, btw the repro uses output: "server":

https://github.com/vchirikov/astro_issue_02/blob/f7c9532f3e31798f6acbbae748c076fe9fe185d9/astro.config.mjs#L7

[vchirikov]

To be clear, the issue isn't about /404 rewriting, it's about wrong 404 status from rewritten page (/about in the gif), which should return 200, but returned 404 instead...

[ematipico]

To be clear, the issue isn't about /404 rewriting, it's about wrong 404 status from rewritten page (/about in the gif), which should return 200, but returned 404 instead...

That wasn't clear from your "expectation" section. Can you be more clear, please? For example why /about should return 200 instead of 404? It's important to voice and justify expectations.

Also, could you reduce the branching of the middleware in your repro? It's not very clear where's the issue.

[vchirikov]

could you reduce the branching of the middleware in your repro?

done in https://github.com/vchirikov/astro_issue_02/tree/issue-11306

Can you be more clear, please?

As a developer I expect that successfully rewritten page should return status 200 instead of 404:

[ematipico]

As a developer I expect that successfully rewritten page should return status 200 instead of 404:

You haven't told me why though, because in this case you're intentionally rewriting with special route 404, which always returns a status code 404. Why should it return a 200? IMHO, it should not.

[vchirikov]

I rewrite all routes to next('/') (see middleware.ts on the screenshot), I don't get your you're intentionally rewriting with special route 404, which always returns a status code 404

I rewrite to an existing page, this page's response.status is 200 (!), I returned the Response object from middleware with status 200 (see the console.log in terminal + blue lines on the screenshot where I show Response with status code 200) but the Astro returns 404 to the browser.

Again rewriting correctly renders index.astro page if I use next('/') but with 404 status code despite the return code from index.astro which is 200 )

---

upd: oh, I think I get your point, I rewrite un-existed page and this is a special 404 route which always returns 404, but:

1. it's unexpected behavior, I explicitly return the Response object with desired status code, but framework implicitly changes the response for me
2. I can do this for another (existed) routes, it's inconsistency
3. there is no docs about it

Example where it is useful:

I want to rewrite /${locale}/${url} to /${url} (and use /${url} as well, without copy-pasting the same pages to [..locale]) page and extract locale to the Astro.locals, with current behavior I will always get 404 on rewritten pages.

[vchirikov]

@ematipico the issue is not fixed in 4.11.1

repro: https://github.com/vchirikov/astro_issue_02/tree/issue-11306

npm run dev
# expected status is 200
curl -I -X GET http://localhost:4321/foo

[FugiTech]

@ematipico Apologies, but I don't believe this is fixed in 4.11.4 or 4.11.5. The repro above still returns 404 on both versions.

I also noted that the repro and your fix uses i18n, but personally I do not believe this issue is limited to using i18n. For example, my app is using rewriting to offer an archive of previous versions of the site under /v1/, /v2/ etc, while / is the latest version. It's very similar, but does not use or enable the i18n module. I'm happy to provide a repro without i18n if needed.

[ematipico]

@FugiTech the fix I applied isn't related to i18n, however that's how the user found the bug and that's how I tested the fix.

Please open a new issue with a new reproduction and we will try to fix your issue too.

[vchirikov]

@ematipico the issue is still here in 4.11.5

p.s. repro the same - https://github.com/vchirikov/astro_issue_02

[vchirikov]

@ematipico reopen please

[bgentry]

@ematipico There are definitely still issues with this. I don't have time at this exact moment to create a reproduction, but I have a super simple middleware that just intercepts some paths to return responses, otherwise passing the requests through:

export const onRequest: MiddlewareHandler = (context, next) => {
  const headers = {"Content-Type": "text/html"}; 
    return new Response(
      `<html>
        <head>
        </head>
        <body>
          <p>Some text</p>
        </body>
      </html>`,
      { headers, status: 200 },
    );
};

Despite the status: 200 in there, the response comes back with the provided body but with a 404 response status.

I will try to create a standalone reproduction this evening.

[ematipico]

I was on vacation. I will look into it.

[vchirikov]

@ematipico I've updated the repro to 4.12.0, now the real http status is 200, but in stdout (logging) the status is 404.

expected response.status is 200
22:20:15 [404] /foo 4ms

[ematipico]

Please open a new issue, the original one is fixed :)