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

Clarification on redirect status code (303) #84

Closed
danjac opened this issue Jan 7, 2021 · 6 comments
Closed

Clarification on redirect status code (303) #84

danjac opened this issue Jan 7, 2021 · 6 comments

Comments

@danjac
Copy link

danjac commented Jan 7, 2021

According to documentation:

https://turbo.hotwire.dev/handbook/drive#redirecting-after-a-form-submission

After a stateful request from a form submission, Turbo Drive expects the server to return an HTTP 303 redirect response, which it will then follow and use to navigate and update the page without reloading.

It appears that Turbo works fine with any 3xx code (e.g. 301 or 302). However is this going to be a hard rule? Many non-Rails frameworks (e.g. Django) assume a 301 or 302 redirect by default, and this will break standard behavior. Is there a good reason for this or should the docs be updated?

@danjac
Copy link
Author

danjac commented Jan 7, 2021

Is it a hard requirement to return a 303 or is any 3** response acceptable? What is Turbo's actual behavior here?

@sstephenson
Copy link
Contributor

It isn't a Turbo requirement, it's an HTTP requirement. The Fetch API follows redirects automatically and doesn't give clients access to the redirect status codes; clients only have access to the status of the final request, plus a flag which says whether a redirect took place.

@johnmaxwell
Copy link

Thanks for the conversation on this. I'm looking into this now.

Before we change all the 302s to 303s in our legacy app we want to know if there is meaningfully different behavior in a Turbo app when responding to non-idempotent requests with 302s vs 303s. As noted, the documentation makes it sounds like it is a Turbo requirement ("... Turbo Drive expects the server to return an HTTP 303 redirect response..."), but the response to this issue sounds like HTTP best practice rather than an operational requirement.

Like @danjac I have found no difference in the way the "Turbo stack" (including its use of fetch here) operates when returning a 302 or a 303 to a non-idempotent request. For example, our login POST endpoint validates credentials and redirects logged in users back to our app homepage. Whether I respond to the login POST request with a 302 or 303, I see no meaningful difference in behavior in the browser or the Rails log: a POST request to the login endpoint of type TURBO_STREAM followed by a GET request to the homepage of type TURBO_STREAM.

I understand that best practice has a role to play in development and the good tools will guide users that. But without an understanding of why 303s are required/expected we risk cargo-culting and misinformation when trying to troubleshoot Turbo integration issues.

Is there a case currently where a Turbo app handles 302 and 303 responses differently for non-idempotent requests?

@KonnorRogers
Copy link
Contributor

KonnorRogers commented Jun 16, 2021

@johnmaxwell So im currently rewriting UJS in typescript, and slammed into this head on. There are definitely different semantic differences.

The issue stems from the Fetch / HTTP spec. Basically if I issue a request like so:

fetch("/posts/5", {method: "delete"})

and /posts/5 redirects to /posts on success, then both requests are made as DELETE requests instead of /posts/5 being a DELETE request and the request to /posts being a GET request.

This happens if the header is not 303. If the header is 303, the first request is DELETE, and the following is GET.

Heres a video of an ajax delete using fetch with the standard 302 headers to show what happens.

non-303-request-header.mov

Also of note, this behavior does not happen with POST, most likely for backwards compatibility of some kind.

  • If one of the following is true:

    • actualResponse’s status is 301 or 302 and request’s method is POST
    • actualResponse’s status is 303 and request’s method is not GET or HEAD
  • then:

    • Set request’s method to GET and request’s body to null.
    • For each headerName of request-body-header name, delete headerName from request’s header list.

https://fetch.spec.whatwg.org/#http-redirect-fetch

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/303

@johnmaxwell
Copy link

johnmaxwell commented Jun 16, 2021

@ParamagicDev Yep, okay. I confirmed that sending using fetch to send DELETE, PATCH, and PUT requests do what you describe when receiving a 302: redirect with the original verb instead of making a subsequent GET request. This is what @sstephenson said above -- and I should have understood it -- but I didn't, and seeing your illustration drove it home. Thanks for taking the time to document.

original method original response redirect method
GET 302 GET
GET 303 GET
POST 302 GET
POST 303 GET
PUT 302 PUT
PUT 303 GET
PATCH 302 PATCH
PATCH 303 GET
DELETE 302 DELETE
DELETE 303 GET

t27duck added a commit to t27duck/rails that referenced this issue Oct 13, 2022
The default controller scaffolding currently sends back a 302 for its
destroy action.

Since Hotwire is the default for Rails going forward, this can be
problimatic. Turbo uses the fetch API internally which is particular on
how it handles redirects.

As outlined in this table in this Turbo issue,
hotwired/turbo#84 (comment),
Turbo making a DELETE request (not POST + hidden _method field) and
recieving a 302 back will result in another DELETE request instead of a
GET request for the redirect.

This updates the controller template used for the scaffold generator
to send back the 303 see other status code for the destroy action.

For consistancy-sake, the Action Controller Overview guide examples
were also updated. The main Getting Started guide page already has
see other used in its example destroy action.

For non-Hotwire users, the browser will still properly redirect on
a 303 as it does with a 302.
dentarg added a commit to dentarg/kartonger that referenced this issue Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants