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

Add redirect::Policy::none to wasm client #2119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ostenbom
Copy link

Solves #2071

https://developer.mozilla.org/en-US/docs/Web/API/Request/redirect

This PR keeps the default fetch redirect functionality as-is (it doesn't set anything, and therefore "follow" is used).

In order to support "manual", we use the Policy::none() interface that does the same thing as the other clients "disables all redirect behaviour".

I deliberately omitted the error kind, since no error interface existed in the current redirect::Policy struct.

Let me know if there's anything else to be improved, thanks!

@seanmonstar
Copy link
Owner

Does this actually return a Response type? When I read through the fetch spec, it sounded like this returns an error with things like status: 0 and such.

@swfsql
Copy link

swfsql commented Feb 19, 2024

To add info on this - in case someone is trying to customize the following on redirects - I naively thought that manual would allow me to "manually" follow the redirects but the fetch API instead just return a response without any data (without the redirection location - so I think the return is not useful in case you do want to follow the redirection in a custom way?).

Found those discussions for the fetch api:

@ostenbom
Copy link
Author

Interesting that there seems to be some differences in implementations here 😅

async function fetchRedirect() {
  const response = await fetch("https://mock.httpstatus.io/307");
  // This will be 200
  console.log({
    status: response.status,
    location: response.headers.get("location"),
  });
  const responseManual = await fetch("https://mock.httpstatus.io/307", {
    redirect: "manual",
  });
  // This will be 307
  console.log({
    status: responseManual.status,
    location: responseManual.headers.get("location"),
  });
}

(async () => {
  await fetchRedirect();
})();
  • When I run this in the node, I get a status 307 on the response + the correct location
  • When I run it in the browser (chrome) I get the status 0 thing
  • When I run this in rust / with this commit + reqwest, I can successfully proxy the 307 status codes, so that the redirects are not followed (no status code 0, no problems with response objects)

I wonder if this could have to do with me running this on cloudflare workers, and that it's cloudflare's implementation of fetch that's allowing this to work?

At the same time it feels strange that this behaviour isn't supported by the fetch spec.. Where do we go from here? Do the implementation differences make this harder to support?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants