Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Astro.locals, set in middleware, available on 404 page in dev mode but not in production mode #8154

Closed
1 task
xriter opened this issue Aug 18, 2023 · 9 comments · Fixed by #8200
Closed
1 task
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: error pages Related to 404 and 500 handling (scope) feat: ssr Related to SSR (scope)

Comments

@xriter
Copy link

xriter commented Aug 18, 2023

What version of astro are you using?

2.10.12

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

npm

What operating system are you using?

Mac

What browser are you using?

Safari

Describe the Bug

After some previous bugs around the 404 page have been solved, I still experienced problems with my 404 page not rendering. I now found the reason: another bug.

When using middleware.ts to set Astro.locals, you can then use those values in the 404.astro page when it's in dev mode. But when in production mode, those values are undefined. (This caused the page to crash without me knowing, resulting in a blank 404 page).

👉 So in short: the behaviour of Astro.locals availability in 404.astro between dev and production mode is inconsistent.

This problem started from Astro 2.9.7. Before that it worked as expected.

To repro and see the difference between the 2 modes, go to the stackblitz link.
Then first run npm run dev and then npm run build && npm run preview

Dev mode (the expected result):
Scherm­afbeelding 2023-08-19 om 01 37 38

Production mode:
Scherm­afbeelding 2023-08-19 om 01 38 03

(This is a continuation of the problem I first described in issue #8054, but turns out to be a separate bug.)

What's the expected result?

I expect Astro.locals, set in middleware.ts to be available in 404.astro in production mode.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-trfvka-twqzu5?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Aug 18, 2023
@lilnasy lilnasy added - P4: important Violate documented behavior or significantly impacts performance (priority) feat: ssr Related to SSR (scope) feat: error pages Related to 404 and 500 handling (scope) and removed needs triage Issue needs to be triaged labels Aug 19, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Aug 19, 2023

Might be related #7881

@lilnasy
Copy link
Contributor

lilnasy commented Aug 19, 2023

@xriter Does middleware work at all for 404?

If not, this is the same issue I brought up in #7881.

@xriter
Copy link
Author

xriter commented Aug 19, 2023

I added some console logs to the middleware in the stackblitz.
The middleware gets called in dev mode, but not in production mode.

@xriter
Copy link
Author

xriter commented Aug 19, 2023

#7881 is older, referring to Astro 2.9.6. In that version the middleware DID get called for custom 404 pages (in both modes), but not for the default 404 page (in both modes). That is what that issue was about: the default 404.

I guess the whole 404 behaviour changed since then, but not for the better.

@lilnasy
Copy link
Contributor

lilnasy commented Aug 19, 2023

Thanks for opening this issue and confirming!

To clarify, this issue is not a duplicate of 7881. When I reported it in that issue, I should've created another issue instead.

@lilnasy
Copy link
Contributor

lilnasy commented Aug 19, 2023

Is the workaround possible in your case?

@xriter
Copy link
Author

xriter commented Aug 19, 2023

Tried it in: https://stackblitz.com/edit/github-trfvka-vfy3p1?file=src%2Fpages%2F%5Bslug%5D.astro
I guess that works.
It doesn't fix the root problem though.
Are there any downsides of using [slug].astro instead of 404.astro that I can't think of right now?

@lilnasy
Copy link
Contributor

lilnasy commented Aug 19, 2023

I don't think there is.

You may already be using a slug route, but in that case, I think 404.astro never gets matched anyway.

@ematipico
Copy link
Member

This fix will be released as part of Astro 3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: error pages Related to 404 and 500 handling (scope) feat: ssr Related to SSR (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants