Change the http_kernel.basic service to use Symfony 6.2's default of catching all throwables

Presume this has an impact on #3051459: Replace error and exception handlers with implementation from Symfony ErrorHandler component?

As SF said this parameter added only to prepare to 7.0 so looks it must have

+++ b/core/tests/Drupal/FunctionalTests/Bootstrap/UncaughtExceptionTest.php
@@ -143,27 +143,6 @@ public function testMissingDependency() {
-    $settings_php .= "set_error_handler(function() {\n";
-    $settings_php .= "  header('HTTP/1.1 418 I\'m a teapot');\n";
-    $settings_php .= "  print('Oh oh, flying teapots');\n";
-    $settings_php .= "  exit();\n";
-    $settings_php .= "});\n";

Without overriding http_kernel.basic, is there a proper place for someone to do this that's not index.php? And not just this example, but the one in the issue summary of #2521852: Make it possible to use your own exception handler of inserting https://booboo.thephpleague.com/ or similar. If the concept of a custom error handler is still a reasonable concept, and it's only that its location needs to be moved from index.php to elsewhere, then could we change this test to that rather than removing it entirely?

To clarify #7, I'm not necessarily advocating for preserving the ability to use set_error_handler(), but rather to accomplish the goal that someone might have for inserting error handling logic. For example, in the case of https://github.com/thephpleague/booboo, maybe what would be needed is instead of index.php calling $booboo->register() which relies on set_error_handler(), the new recommended practice might be to add an event subscriber or middleware that calls $booboo->errorHandler() directly? I just want to make sure we have some clarity on what that new recommended practice should be, and ideally include that in a test and in the change record.

For example, in the case of https://github.com/thephpleague/booboo

Or maybe it would make more sense to use https://github.com/symfony/error-handler as the reference example.

The correct way of handling errors inside Symfony is to listen for the KernelEvents::EXCEPTION event.

Makes sense. In that case, it seems like the difference that Symfony 6.2 introduces is that HttpKernel will now dispatch this event for all throwables, not just exceptions. So I think that the CR for this issue, whether that's https://www.drupal.org/node/3305024 or a new one, should show a before/after where before you used set_error_handler() in index.php, and the after is where you can now add a KernelEvents::EXCEPTION subscriber that works for errors that are not exceptions.

Or if you want to handle them outside of Symfony, you can remove core's exception.final service and use set_error_handler()

From a BC standpoint, if you wanted to keep an existing set_error_handler() in index.php working the way it did in Drupal 9.5, what you'd probably want to do is not remove the exception.final service, but rather override it with a subclass that only calls the parent class if $event->getThrowable() is an instance of \Exception. Would it make sense to mention that in https://www.drupal.org/node/3305024 as something that we would recommend trying before deciding to override the http_kernel.basic service's arguments?

#13 is about change records. For the purposes of what this issue does with respect to test coverage, +1 to #10: I think it makes a lot of sense for the test coverage of a custom error handler to remove the exception.final service. The fact that #10 failed in another test is a bit concerning: that would seem to point to either changes to \Drupal::state() or to the event dispatcher persisting across tests.

Alternatively, instead of having the test remove the exception.final service, we could make the test symmetrical with testUncaughtExceptionCustomExceptionHandler() by triggering the missing dependency error from middleware (MonkeysInTheControlRoom) rather than in the controller (LonelyMonkeyController). I think this might actually be the better way to test it, since then the use case of set_error_handler() is clearer: it's not necessarily that it's undesirable to handle errors in KernelEvents::EXCEPTION, it's that there are code paths, like middleware, for which KernelEvents::EXCEPTION isn't dispatched.

This implements #15, but it's failing locally for me, and I don't know yet if that's a quirk of my machine, a quirk of the test, or if there's something about the flow of error handling that I'm misunderstanding.

patch fails to apply, probably because HTTPS endpoint in Gitlab is out of sync with SSH one

18:42:40 Performing git checkout of https://git.drupalcode.org/project/drupal.git -b 10.0.x to /var/lib/drupalci/workspace/jenkins-drupal_patches-154211/source.
18:42:40 Git Command: sudo -u www-data git clone -v -b 10.0.x  https://git.drupalcode.org/project/drupal.git '/var/lib/drupalci/workspace/jenkins-drupal_patches-154211/source'
18:42:40 sudo -u www-data git clone -v -b 10.0.x  https://git.drupalcode.org/project/drupal.git '/var/lib/drupalci/workspace/jenkins-drupal_patches-154211/source'

We've made a chance to redirect https Git traffic to our primary while we sort out the issue with our git secondary lagging... requeued the patch tests in #17.

No luck on requeues... we'll continue to work on this.

The reason #17 fails for me locally is that even though \Drupal::service('broken_class_with_missing_dependency') throws an ArgumentCountError which is not an Exception, _drupal_exception_handler() still runs, which preempts the error handler from running. But I'm not clear on why that's different from earlier Symfony versions. In other words, how does testMissingDependencyCustomErrorHandler() currently pass tests in HEAD without running into that?

Patches are now able to get past the 'fail to apply' due to GitLab replication mismatch stage - so I will bow out from here :)

In other words, how does testMissingDependencyCustomErrorHandler() currently pass tests in HEAD without running into that?

Ah, it's because the custom error handler isn't invoked in that test in HEAD either. Instead, #2599446: UncaughtExceptionTest fails on PHP7 changed MonkeysInTheControlRoom to catch the \TypeError and just do the same thing with it as what the test's custom error handler does.

In that case, my suggestion is to remove testMissingDependencyCustomErrorHandler(), since a missing dependency is a thrown/caught error that doesn't involve an error handler, and to instead to add a test to ErrorHandlerTest that tests a custom error handler for something that isn't thrown/caught, such as a PHP warning. This patch does that.

Updated the issue title and summary accordingly.

Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!

Thank you!