QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.68k stars 1.29k forks source link

[🐞] Cannot `send` a redirect response #3231

Open zhuhaow opened 1 year ago

zhuhaow commented 1 year ago

Which component is affected?

Qwik City (routing)

Describe the bug

We cannot send a redirect response with send.

export const onGet: RequestHandler = async ({ send }) => {
  const response = await fetch(from_somewhere); // If this is a redirect response.
  await send(response);
};

This would return a 404 response.

Reproduction

https://stackblitz.com/edit/qwik-starter-nnzpec?file=src/routes/index.tsx

Steps to reproduce

No response

System Info

System:
    OS: macOS 13.2.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 82.17 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    npm: 8.5.0 - ~/.nvm/versions/node/v16.14.2/bin/npm
  Browsers:
    Chrome: 110.0.5481.177
    Safari: 16.3
  npmPackages:
    @builder.io/partytown: ^0.7.5 => 0.7.5
    @builder.io/qwik: 0.20.1 => 0.20.1
    @builder.io/qwik-city: 0.5.2 => 0.5.2
    undici: ^5.20.0 => 5.20.0
    vite: 4.1.4 => 4.1.4

Additional Information

No response

stackblitz[bot] commented 1 year ago

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

zanettin commented 1 year ago

Hi @zhuhaow

Thank you for creating this issue 🙏 TBH i am not sure what you want to achieve with the code 👼 .

When the plan is to redirect the user to another route:

import { type RequestHandler } from "@builder.io/qwik-city";

export const onGet: RequestHandler = async ({ redirect }) => {
  throw redirect(307,'https://google.com', );
};

or like mentioned here: https://qwik.builder.io/qwikcity/guides/redirects/#redirects

If you want to have a reverse proxy, this could help: https://qwik.builder.io/qwikcity/endpoints/#create-a-reverse-proxy-using-a-fetch

thank you for further info about your use case 🙏

zhuhaow commented 1 year ago

I have some request handlers that I want to share with multiple projects built with remix and qwik. I try to return a redirect response following standard Web API so I don't need to provide any adapters for remix and qwik.

export const onGet: RequestHandler = async ({ request, send }) => {
  const response = await some_handler(request) // This could be a redirect response.
  await send(response);
};

Of course, it's still possible to work around this by passing an extra redirect function to the handler for different frameworks.

But even in the document on reverse proxy you mentioned, it still won't work as a reverse proxy correctly since it won't forward redirection. The below code won't work and returns a 404 response.

send(
    await fetch(
      "https://httpbin.org/redirect-to?url=https%3A%2F%2Fgoogle.com&status_code=307"
    )
  );

Maybe I'm missing something here. But I don't get why we need a redirect method given it's so easy to write send(Response.redirect(url, statusCode)); return;. And it's quite confusing when should we throw and when should we not. E.g., https://qwik.builder.io/qwikcity/endpoints/#changing-the-status-code we have to throw to set the status code. And throw on a redirect also makes little sense since we may want to customize the header.

manucorporat commented 1 year ago

Agree this is a legit issue! send() should work in this case, should be an easy fix!

zhuhaow commented 1 year ago

Awesome! Looks forward to the fix

olegKusov commented 1 year ago

Is there a way to simplify reverse proxy? With docs I don't get how this reverse proxy works. In docs example ( https://qwik.builder.io/qwikcity/endpoints/#create-a-reverse-proxy-using-a-fetch) with simple URL without search params, without body.

And it will be good if we could use standard NodeJS req, res with http-proxy package for example.

import { RequestHandler } from "@builder.io/qwik-city"
var httpProxy = require('http-proxy');

var proxy = httpProxy.createProxyServer(); 

export const onGet: RequestHandler = async ({request, response}) => {
    proxy.web(reqest, response, { target: 'http://mytarget.com:8080' });

}