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

Confusing hybrid mode on Cookie #7215

Closed
jamesli2021 opened this issue Mar 6, 2024 · 11 comments
Closed

Confusing hybrid mode on Cookie #7215

jamesli2021 opened this issue Mar 6, 2024 · 11 comments
Labels
help - leave feedback Let's crowd source this one! Looking for comments, suggestions, LGTMs! help wanted Issues looking for someone to run with them! improve documentation Enhance existing documentation (e.g. add an example, improve description)

Comments

@jamesli2021
Copy link
Contributor

📝 Issue Description

Unclear about hybrid mode.

📋 On which page(s) it occurs

https://docs.astro.build/en/guides/server-side-rendering/#on-demand-rendering-features

🤔 Expected Behavior

What do the readers understand from this?

Extract from doc:
In server and hybrid (with opt-out prerender) modes, a page or API endpoint can check, set, get, and delete cookies.

👀 Current Behavior

When I set to output: hybrid in astro config file, the terminal can confusing for newbies, I know we need to opt-out of pre-render. But this warning need to be clear.

4:15:15 [WARN] `Astro.request.headers` is not available in "static" output mode. To enable header access: set `output: "server"` or `output: "hybrid"` in your config file.

🖥️ Browser

NA

📄 Additional Information

No response

@jamesli2021 jamesli2021 added the bug Something on the site isn't working label Mar 6, 2024
@sarah11918
Copy link
Member

Thank you for raising this @jamesli2021 ! I agree and it's actually on my list to convert some of our wording over from "modes" (server, hybrid) to the actual individual pages themselves, because you can have pre-rendered modes in ANY mode, and yes, that is what the error message should be explaining. You might have one of the correct modes configured, but it will only work on a page that's rendered on-demand.

@Princesseuh - I went to look in https://github.com/withastro/astro/blob/main/packages/astro/src/core/errors/errors-data.ts for this error but I'm having trouble finding it. Is it possible we don't have a docs entry for this one?

In general, I think we'll make a task to do a sweep of SSR related errors and make sure they refer to the specific endpoint/route being prerendered or not, vs the output mode, since that doesn't guarantee that every page uses SSR.

@sarah11918 sarah11918 added improve documentation Enhance existing documentation (e.g. add an example, improve description) help wanted Issues looking for someone to run with them! help - leave feedback Let's crowd source this one! Looking for comments, suggestions, LGTMs! and removed bug Something on the site isn't working labels Mar 18, 2024
@Princesseuh
Copy link
Member

We only have pages for errors, this seems to be a random warning in the console, so it's most likely somewhere random in Astro.

@sarah11918
Copy link
Member

Alright, treasure hunt, it is! Let's find this warning in the Astro code base 😅

@mingjunlu
Copy link
Contributor

@lschierer
Copy link
Contributor

I'd really like to see this fixed, it is one of a couple of warnings that gets spewed everywhere in my console when I run a build and makes it much harder to find actual problems.

@sarah11918
Copy link
Member

I think this was already being looked into as an overly aggressive message in the first place, but that doesn't address the text of the error, so I will mention @ematipico and @matthewp here to ask whether an update to the message linked by @at-the-vr makes sense:

Instead of saying "not available in static ouput MODE", this maybe should be "this cannot be used on static prerendered PAGES" (because they may very well have server or hybrid output mode enabled, so the hint doesn't really get at the problem)? Then, the hint would be to make sure the page itself is rendered on demand, and not prerendered.

@ematipico
Copy link
Member

Yeah, it makes sense to update the message

@sarah11918
Copy link
Member

It looks like the most recent version of this file has an updated error message that addresses this:

https://github.com/withastro/astro/blob/bab700d69085b1de8f03fc1b0b31651f709cbfe3/packages/astro/src/core/request.ts#L80

\Astro.request.headers` is unavailable in "static" output mode, and in prerendered pages within "hybrid" and "server" output modes. If you need access to request headers, make sure that `output` is configured as either `"server"` or `output: "hybrid"` in your config file, and that the page accessing the headers is rendered on-demand.`

So, I will close this issue since the message has in fact been updated since the original issue was filed! 🙌

@firxworx
Copy link

@sarah11918 I have been playing around with Astro + Lucia auth and this warning fills my console when I have the dev server running.

For auth I added a middleware check to verify that Origin + Host header of the request match for any requests where the context.request.method !== 'GET'. This the only part of the entire codebase that actually checks Astro.request.headers so I assume this is what's triggering the warning.

Astro is consistently cluttering the dev console with the warning even if I set the default output to server in my Astro config.

I agree with OP this is confusing especially because it appears to contradict a config setting that is clearly set. This makes it look like a bug in the project and risks sending the dev on a wild goose chase or it looks like a bug in Astro.

Even the "fixed" message is just as confusing -- if not worse -- given the context of this particular scenario.

I think as Astro expands more into server-side stuff more devs are going to be doing things like this and encountering this issue.

I haven't worked a ton with SSR yet with Astro so I'm not sure if in middleware there's a way to detect if the request is related to something where the server does "work" or where it doesn't and the request is for a statically generated page. It would be nice to know for conditional handling or messaging.

That said, in general, I think we still very much do want to check cookies and headers for "static" resources that are supposed to be restricted and require auth. The point of running server-side code is to check each request in case the client isn't allowed to receive the response!

@ematipico
Copy link
Member

ematipico commented Jun 21, 2024

I will bring this up to the team, this is a valid concern and we need to find the correct way to address it.

@ematipico
Copy link
Member

ematipico commented Jun 21, 2024

I haven't worked a ton with SSR yet with Astro so I'm not sure if in middleware there's a way to detect if the request is related to something where the server does "work" or where it doesn't and the request is for a statically generated page. It would be nice to know for conditional handling or messaging.

You can do that manually by looking at context.url, and decide to trigger the SSR logic based on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help - leave feedback Let's crowd source this one! Looking for comments, suggestions, LGTMs! help wanted Issues looking for someone to run with them! improve documentation Enhance existing documentation (e.g. add an example, improve description)
Projects
None yet
Development

No branches or pull requests

7 participants