Problem/Motivation

Follow-up to #2230121: Remove exit() from FormBuilder and #2503429: [PP-*] Allow both AJAX and non-AJAX forms to POST to dedicated URLs.

Drupal currently supports throwing an exception to exit the code flow and force an immediate redirect. This was implemented largely as a stop gap to allow the removal of direct response rendering and exits from core API's allowing better leverage of Symfony's HTTP stack.

Those API's should be refactored to allow common exit flows or redirecting without throwing an exception and then the Exception and its handling can hopefully be deprecated and retired.

Remaining tasks

User interface changes

N/A

API changes

Comments

andypost’s picture

Why this issue postponed til 9.x?
maybe there's a way to make it in 8.x by providing some BC

catch’s picture

Version: 9.x-dev » 8.0.x-dev
Priority: Critical » Major
Status: Postponed » Active

Yes this would be a major performance improvement, so I'd consider it at any point during 8.x if we can do it without causing massive disruption to contrib modules.

Wim Leers’s picture

Issue tags: +Performance, +D8 cacheability

This is also why render caching is disabled for POST requests. Which means that on every single POST request, we re-render a lot of things that with this issue in, could simply be retrieved from the render cache.

neclimdul’s picture

My goodness guysy'all, we use an exception for code flow in formapi?! This was a mess to decipher!

So, its turns out the whole rigamarole currently in FormAPI where we throw an exception, catch that exception, then rewrite that exception breaks when you try to use the HttpKernelInterface::handle() flag to turn off exception catching and view a normal PHP/xdebug backtrace. I don't know if that makes this a bug or some related issue but using an exception like this for normal code flow is very broken.

catch’s picture

Status: Active » Closed (duplicate)

Marking duplicate of #2503429: [PP-*] Allow both AJAX and non-AJAX forms to POST to dedicated URLs. This is the original issue, but that one has a more up-to-date plan on it.

catch’s picture

Title: Separate code path used for form submission from form rendering » Deprecate EnforcedResponseException support
Version: 8.0.x-dev » 8.2.x-dev
Issue summary: View changes
Status: Closed (duplicate) » Postponed

Once #2503429: [PP-*] Allow both AJAX and non-AJAX forms to POST to dedicated URLs is fixed, and all core forms are submitted, we'll be able to deprecate support for EnforcedResponseException for 9.x. This is a case where a contrib module update to use the new API could take seconds off a form submission so worth prodding.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

fubhy’s picture

I need to override this behavior for caching specific POST requests (GraphQL queries). Right now this is a PITA because I have to override the entire method bodies for get()/set(). Can we move this hotfix into a helper method on the RenderCache / PlaceholderingRenderCache classes at least?

Fabianx’s picture

#8: Sure, could you open an issue for that?

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ndobromirov’s picture

Issue summary: View changes
Issue tags: +DrupalWTF

Sorry to add the tag but I got here after getting a bug caused by this exception used for code flow.

I was not expecting that Form-API will throw an exception to handle the submit redirect.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

We could also add a redirect middleware ala #3214949: Headers have already been sent after upgrade to Drupal 9.2 (can't login) and make that a supported API

The code there could easily be lifted to core

bradjones1’s picture

Related in other contrib: Commerce uses (abuses?) this for redirects:

https://git.drupalcode.org/search?utf8=%E2%9C%93&search=enforcedresponse...

neclimdul’s picture

Issue summary: View changes

@larowlan definitely seems like a possibility. I think I've done something similar in an event listenert where I needed to modify the response headers not replace the response. You have to play carefully with page cache but I believe its maybe possible here too without adding another middleware?

Reaching back into the past, I have to wonder if part of the reason for the implementation was more about "exiting" instead of the actual redirecting. Like if you're in a build or validation form step and need to redirect _before_ any additional code can run. I think that's a failing of API design. No offense intended, we can only solve so many problems at once and we didn't fully know what a lot of D8 was going to look like. I certainly didn't offer a better option.

At this point seems like that's probably solvable by paying down some technical dept on those API's. I do wonder if we've created one of those "this feature exists but if you're using it you're doing it wrong" sort of features we'll have to continue support for contrib for a while though.

@bradjones1 Good find. I'm not currently super familiar with Commerce's code but skimming, it seems to into this exact problem in its own API design. Their main use seems to be in its wizard builder system where plugins are responsible for a lot of validation of "should the user be here" and then triggering a "no they should be on another step" redirects. The design requires this to happen primarily in the _build_ step, and that can only return form array. They use the exception as a path out that provides a "secondary" return type but also a way of exiting the code flow.

Its clever and convenient but probably not the best approach because of the exception but also it mixes a lot of concerns in the build method. This also seems solveable but probably requires a lot of planning and API design. If I can naively suggest a solution, this seems like a common pattern on their plugins, so a method for prevalidation(and a new interface?) that can return a step id, or a redirect response, or maybe a magic step id redirect response. This would also probably allow for the consolidation of a lot of patterns and boiler plate into commerce itself instead of in the plugin build methods.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

gabesullice’s picture

*nods* knowingly to the next poor soul to land on this issue after an hour or two of some crazy step debugging.

We share a special bond now.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

catch’s picture

#3395776: Make POST requests render cacheable is making POST requests render cacheable without changing form discovery, which doesn't affect this issue except it means that #2503429: [PP-*] Allow both AJAX and non-AJAX forms to POST to dedicated URLs and this issue are more clean-up than performance improvements now.