Render 400-500 status response HTML #39

seanpdoyle · 2020-12-26T02:49:07Z

Related to hotwired/turbo-rails#12
Related to hotwired/turbo-rails#34

Typically, Turbo expects each FormSubmission request to result in a
redirect to a new Location.

When a FormSubmission request fails with an HTTP Status code between
400-499 or 500-599 (e.g. unprocessable entity), render the
response HTML. This commit brings the same behavior to <turbo-frame>
elements.

seanpdoyle · 2020-12-26T02:49:27Z

src/core/drive/form_submission.ts

@@ -99,7 +99,9 @@ export class FormSubmission {
  }

  requestSucceededWithResponse(request: FetchRequest, response: FetchResponse) {
-    if (this.requestMustRedirect(request) && !response.redirected) {
+    if (response.unprocessableEntity) {


Is this the only special case status we want to support?

It might make sense to cover all 4xx errors, not only 422. 422 is pretty specific to rails scaffolding conventions where as 4xx is more widely used for forms that have errors.

422 is problematic also for Rails. Default HTML response for invalid form is 200, 422 is usually used only for JSON responses. I know it can be changed, but support for default behaviour would be nicer :).

Example from rails scaffold generator:

# POST /rooms # POST /rooms.json def create @room = Room.new(room_params) respond_to do |format| if @room.save format.html { redirect_to @room, notice: 'Room was successfully created.' } format.json { render :show, status: :created, location: @room } else format.html { render :new } format.json { render json: @room.errors, status: :unprocessable_entity } end end end

src/core/drive/navigator.ts

Intrepidd · 2020-12-26T09:38:49Z

Note : I realised that form submits sent by turbo don't have the Turbo-Frameheader. This header helps telling the backend this is a frame request. Rails for instance doesn't render the layout if this is a frame request. Should we send this header now that we accept a frame response back ?

santib · 2020-12-28T12:00:08Z

src/core/drive/form_submission.ts

-    if (this.requestMustRedirect(request) && !response.redirected) {
+    if (response.unprocessableEntity) {
+      this.delegate.formSubmissionFailedWithResponse(this, response)
+    } else if (this.requestMustRedirect(request) && !response.redirected) {
      const error = new Error("Form responses must redirect to another location")


What is the technical reason why we need to have this error triggered when the response is not a redirect?

What is the technical reason why we need to have this error triggered when the response is not a redirect?

#22 (comment)

sstephenson

Nice work. I agree with your choice to expand this to cover all 4xx and 5xx responses.

src/core/drive/navigator.ts

Related to hotwired/turbo-rails#12 Related to hotwired/turbo-rails#34 Typically, Turbo expects each FormSubmission request to result in a redirect to a new Location. When a FormSubmission request fails with an HTTP Status code between 400-499 or 500-599 (e.g. [unprocessable entity][422]), render the response HTML. This commit brings the same behavior to `<turbo-frame>` elements. [422]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422

javan · 2020-12-30T22:26:37Z

src/core/drive/navigator.ts

+    const responseHTML = await fetchResponse.responseHTML
+
+    if (responseHTML) {
+      debugger


Intrepidd · 2021-01-05T14:35:28Z

For long forms, page is not scrolled to the top after submit, do you think this should be fixed natively in turbo ?

When responding to an HTML request that had errors on the resource, responders would not set any status, which means Rails would default to `200 OK`. This has been long working like this but also a long discussion in responders to change that behavior, to be more inline with how other types of requests (like JSON/XML) handle responses by setting the HTTP status to 422 Unprocessable Entity when responding with an error. More recently with the Turbo/Hotwire library, they've added support to render form responses as errors with 4xx statuses [1], so it makes even more sense for responders to make the move here. This change now makes it respond with 422 Unprocessable Entity in such cases when the resource has errors, and should work more out of the box with Turbo/Hotwire. Please note that this is a possible breaking change if you're relying on the previous status to trigger any behavior for errors in your app. [1] hotwired/turbo#39

When responding to a non-GET HTML request that had errors on the resource, responders would not set any status, which means Rails would default to `200 OK`. This has been long working like this but also a long discussion in responders to change that behavior, to be more inline with how other types of requests (like JSON/XML) handle responses by setting the HTTP status to 422 Unprocessable Entity when responding with an error. More recently with the Turbo/Hotwire library, they've added support to render form responses as errors with 4xx statuses [1], so it makes even more sense for responders to make the move here. This change now makes responders use the 422 Unprocessable Entity status in such cases when the resource has errors, and should work more out of the box with Turbo/Hotwire. Please note that this is a possible breaking change if you're relying on the previous status to trigger any behavior for errors in your app. [1] hotwired/turbo#39

…appropriate HTTP status code (dunglas) This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [FrameworkBundle] Add renderForm() helper setting the appropriate HTTP status code | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes  | Deprecations? | no  | Tickets | n/a | License | MIT | Doc PR | todo A 422 HTTP status code should be returned after the submission of an invalid form. Some libraries including [Turbo](hotwired/turbo#39) rely on this behavior and will not display the updated form (containing errors) unless this status code is present. Rails also [recently switched to this behavior ](rails/rails#41026) by default for the same reason. I propose to introduce a new helper method rendering the form and setting the appropriate status code. It makes the code cleaner: ```php // src/Controller/TaskController.php // ... use Symfony\Component\HttpFoundation\Request; class TaskController extends AbstractController { public function new(Request $request): Response { $task = new Task(); $form = $this->createForm(TaskType::class, $task); $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { $task = $form->getData(); // ... return $this->redirectToRoute('task_success'); } return $this->renderForm('task/new.html.twig', $form); } } ``` Commits ------- 4c77e50 [FrameworkBundle] Add renderForm() helper setting the appropriate HTTP status code

This is required by Turbo [1]. This will be the default in Rails 7 [2]. [1] hotwired/turbo#39 [2] rails/rails@2afc905

See: hotwired/turbo#39

seanpdoyle requested a review from sstephenson December 26, 2020 02:49

seanpdoyle commented Dec 26, 2020

View reviewed changes

src/core/drive/navigator.ts Outdated Show resolved Hide resolved

santib reviewed Dec 28, 2020

View reviewed changes

seanpdoyle requested a review from javan December 28, 2020 13:47

jorgemanrubia mentioned this pull request Dec 29, 2020

Add supports for render in controllers turbolinks/turbolinks-rails#40

Open

dsazup mentioned this pull request Dec 29, 2020

Forms don't have to redirect #32

Closed

seanpdoyle changed the title ~~Render 422 Response HTML~~ Render 400-500 status response HTML Dec 29, 2020

sstephenson suggested changes Dec 30, 2020

View reviewed changes

src/core/drive/navigator.ts Outdated Show resolved Hide resolved

seanpdoyle force-pushed the form-submission-422 branch from fa1b6c2 to e73f9e8 Compare December 30, 2020 14:47

seanpdoyle force-pushed the form-submission-422 branch from e73f9e8 to 4670f2b Compare December 30, 2020 15:05

sstephenson approved these changes Dec 30, 2020

View reviewed changes

sstephenson merged commit 4227383 into main Dec 30, 2020

sstephenson deleted the form-submission-422 branch December 30, 2020 21:38

javan reviewed Dec 30, 2020

View reviewed changes

This was referenced Jan 6, 2021

Use 422 instead of 200 as the status code for form submission error responses rails/rails#41026

Merged

Respond form submission errors with 422 status code heartcombo/devise#5325

Closed

nyrf mentioned this pull request Jan 7, 2021

fail when render status: :unprocessable_entity SimoTod/alpine-turbo-drive-adapter#29

Closed

kaspth mentioned this pull request Jan 7, 2021

Missing Turbo-Frame header when submitting a form #86

Closed

nyrf mentioned this pull request Jan 7, 2021

turbo:load won't be triggered when a FormSubmission request fails #85

Closed

dunglas mentioned this pull request Jan 14, 2021

[FrameworkBundle] Add renderForm() helper setting the appropriate HTTP status code symfony/symfony#39843

Merged

carlosantoniodasilva mentioned this pull request Jan 16, 2021

Respond with 422 Unprocessable Entity for non-GET HTML/JS requests with errors heartcombo/responders#223

Merged

pocheptsov mentioned this pull request Apr 27, 2021

Editing a form within a turbo frame - issues #261

Closed

airblade added a commit to airblade/quo_vadis that referenced this pull request Jun 1, 2021

Use 422 status code for form submission error responses

f83c20b

This is required by Turbo [1]. This will be the default in Rails 7 [2]. [1] hotwired/turbo#39 [2] rails/rails@2afc905

fwilkens mentioned this pull request Jul 26, 2021

Absence of turbo:load event for 4xx html responses #325

Closed

janko mentioned this pull request Jul 27, 2021

Turbo frame doesn't render response HTML if status is 4xx or 5xx #326

Open

toddsundsted added a commit to toddsundsted/ktistec that referenced this pull request Sep 11, 2021

Return 4XX status codes on form submission errors.

43e904b

See: hotwired/turbo#39

pythonandchips mentioned this pull request Oct 6, 2021

Add option to disable turbo for form or optin to forms. #419

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render 400-500 status response HTML #39

Render 400-500 status response HTML #39

seanpdoyle commented Dec 26, 2020 •

edited

Loading

seanpdoyle Dec 26, 2020

dylanjha Dec 26, 2020

edariedl Dec 29, 2020

Intrepidd commented Dec 26, 2020

santib Dec 28, 2020 •

edited

Loading

sstephenson Dec 30, 2020

sstephenson left a comment

javan Dec 30, 2020

seanpdoyle Dec 30, 2020

seanpdoyle Dec 30, 2020

Intrepidd commented Jan 5, 2021

Render 400-500 status response HTML #39

Render 400-500 status response HTML #39

Conversation

seanpdoyle commented Dec 26, 2020 • edited Loading

seanpdoyle Dec 26, 2020

Choose a reason for hiding this comment

dylanjha Dec 26, 2020

Choose a reason for hiding this comment

edariedl Dec 29, 2020

Choose a reason for hiding this comment

Intrepidd commented Dec 26, 2020

santib Dec 28, 2020 • edited Loading

Choose a reason for hiding this comment

sstephenson Dec 30, 2020

Choose a reason for hiding this comment

sstephenson left a comment

Choose a reason for hiding this comment

javan Dec 30, 2020

Choose a reason for hiding this comment

seanpdoyle Dec 30, 2020

Choose a reason for hiding this comment

seanpdoyle Dec 30, 2020

Choose a reason for hiding this comment

Intrepidd commented Jan 5, 2021

seanpdoyle commented Dec 26, 2020 •

edited

Loading

santib Dec 28, 2020 •

edited

Loading