[🐞] response.redirect doesn't work as expected

[lengzuo]

Which component is affected?

Qwik City (routing)

Describe the bug

I have a onGet server in layout.tsx to check for authentication for all the pages that used the same layout. I use a query string isLoggedIn as a flag to check for authentication. However, if i navigate to other protected page without isLoggedIn using <Link> or useNavigate(), the authentication check in layout.tsx should bring me back to my authentication page (/auth) as login is required, rather than allow me to stay in my protected page without isLoggedIn.

Please see the sample project from here

Reproduction

https://stackblitz.com/edit/qwik-starter-ul3hqf?file=src/routes/(main)/layout.tsx

Steps to reproduce

1. Run npm run dev.
2. Click the login button
3. U will be redirect to main page with isLoggedIn querystring in the url.
4. Click Other Page.
5. U will be redirect to Other Page without isLoggedIn. But u should be bring to /auth page for authentication without isLoggedIn flag.

System Info

N/A

Additional Information

No response

[stackblitz[bot]]

Start a new pull request in StackBlitz Codeflow.

• qwik-starter-ul3hqf (Open reproduction)

[zanettin]

Hi @lengzuo Thanks for opening this issue 🙏 not really sure if this is a bug or an expected behavior. by analyzing the network tab you can see, that the q-data.json request which returns the needed data for your SPA is actually redirected and returns the data from the /auth route. so the redirect does work. the issue imo is, that instead of a redirect on the server you should return any data via the q-data.json (just return an object from the onGet fn) and do a client side redirect instead. as an alternative you could use the MPA behavior by changing the <Link /> component to regular <a> tag.

just my opinion and not sure what the core team thinks about it and what maybe changes with the new endpoint handling which just got merged (2h ago: https://github.com/BuilderIO/qwik/pull/2394). video shared on loom: https://www.loom.com/share/1252a0e285684bc095e872efd5e1e566

[zanettin]

Hi @lengzuo 👋 Just wanted to check what the state of this issue is. Have you been able to solve it or try out the latest beta of qwik-city using the loader$ fn? I've created a small migration guide over here #2560.

[adaliszk]

After finding out how the new way works in #2560, I wanted to do a redirect. I had changed my onGet to the new format, but it seems that the redirect hook does not work.

My src/routes/index.tsx:

import { RequestHandler } from '@builder.io/qwik-city'

export const onGet: RequestHandler = async ({redirect}) => {
    redirect(307, '/projects/')
}

There is no console output; the page just infinitely loads as if it never received a response. Same with a non-async version:

import { RequestHandler } from '@builder.io/qwik-city'

export const onGet: RequestHandler = ({redirect}) => {
    redirect(307, '/projects/')
}

When I swap around and set the header myself, then it does work:

import { RequestHandler } from '@builder.io/qwik-city'

export const onGet: RequestHandler = async ({send, headers}) =>{
    headers.set('Location', '/projects/')
    send(307, 'Redirecting to /projects/')
}

My environment:

• Node 18.12.1
• Vite 4.0.4
• Qwik-City 0.1.0-beta9
• Qwik 0.16.2

[genki]

For loader$, it seemed that follows the redirect location to fetch data if throw redirect(...). But if just call redirect(...) then the browser is navigated to the location. Is this the spec? In the former case, if the location is absolute URL then an error is caused in renderQData that the RedirectMessage is not serializable. Looking into the source, the throw redirect(...) doesn't expect absolute URL.

https://github.com/BuilderIO/qwik/blob/681ed8b8ab9e2a071901c8a0acb086def46829b5/packages/qwik-city/middleware/request-handler/resolve-request-handlers.ts#L340-L346

https://github.com/BuilderIO/qwik/blob/681ed8b8ab9e2a071901c8a0acb086def46829b5/packages/qwik-city/middleware/request-handler/resolve-request-handlers.ts#L374-L384

[gioboa]

throw redirect(...) Is the right way to go. I don't understand if there are other problem to solve or if this issue is solved. cc @lengzuo

[adaliszk]

Why would a throw be the right way to do anything but errors? Throwing is for errors; a redirect is not an error but just an abbreviation for HTTP headers.

Within loader$, it makes sense that it redirects the page, as it is supposed to send HTTP headers to tell the browser to redirect a page. The throw within that is weird, and that should be reported as a new issue. Returning a redirect would make more sense there, as that is its normal behavior with data already; however, at the moment, I don't think there is any difference between the redirect in RequestHandler and within the routeLoader$.

As for this issue, we should update the example code to the latest version and see if it works now. I haven't used this feature lately.

[adaliszk]

Okay, after checking the docs of how it should be done, errors and redirects are expected to be thrown. So, in terms of documentation and expected code, there are no issues here. However, I would still question why we abuse the exception system for this purpose, but that is a separate discussion.

[gioboa]

I can understand your concern. I close this issue because it's working as expected.