Make it possible to use your own exception handler

+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -612,7 +612,9 @@ protected function handleException(\Exception $e, $request, $type) {
+        set_exception_handler($handler = set_exception_handler('var_dump'));

As of PHP 5.5 you may pass NULL to set_exception_handler and still get back the current handler.

How about just rethrow unless there is a HttpException? This would be analogous to what HttpKernel does if no subscriber handled the exception.

Curious: Why? What's the use case here? That should get added to the issue summary to justify a Major.

Test only patch equals interdiff.

Rebase succeeded with auto-merging. Only some context lines changed.

Reroll.

Beter DX is normal, not major. But I don't say no to this patch.

1. +++ b/core/lib/Drupal/Core/DrupalKernel.php
   @@ -610,24 +610,7 @@ protected function handleException(\Exception $e, $request, $type) {
   +      throw $e;
   

   Nit: We should document @throws in the docblock now.

2. +++ b/core/modules/system/src/Tests/System/UncaughtExceptionTest.php
   @@ -90,17 +90,77 @@ public function testUncaughtException() {
   +    $settings_php .= "  print('Oh oh, flying teapots');\n";
   +    $settings_php .= "  header('HTTP/1.1 418 I\'m a teapot');\n";
   ...
   +    $settings_php .= "  print('Oh oh, flying teapots');\n";
   +    $settings_php .= "  header('HTTP/1.1 418 I\'m a teapot');\n";
   

   Is this ordered this way to ensure output buffering? If there isn't a test specifically for that we might want to document it here so it isn't accidentally removed later to "correct" the order.

Is this ordered this way to ensure output buffering?

Uh, well, that only worked because output_buffering is turned on on most PHP installations (including test bots).

Yeah, but the patch looks great! :)

Huh!? I do not even have a working copy of coder_sniffer on my disk. Maybe that's a sign :)

Interesting detail: interdiff_14927.txt and interdiff_14928.txt are the same files.

+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -610,24 +613,7 @@ protected function handleException(\Exception $e, $request, $type) {
-        $message = Settings::get('rebuild_message', 'If you have just changed code (for example deployed a new module or moved an existing one) read <a href="https://www.drupal.org/documentation/rebuild">https://www.drupal.org/documentation/rebuild</a>');
-        if ($message && Settings::get('rebuild_access', FALSE)) {
-          $rebuild_path = $GLOBALS['base_url'] . '/rebuild.php';
-          $message .= " or run the <a href=\"$rebuild_path\">rebuild script</a>";
-        }
-        print $message;

Are we sure we want to remove this? How are we sure this code is dead?

The only way to get there is when an exception is thrown while handling an exception in _drupal_exception_handler() or _drupal_log_error() (i.e. if the exception handler itself is broken). This was a quite common case before #2509898: Additional uncaught exception thrown while handling exception after service changes, but is now fixed.

In my opinion there is no meaningful message we can display when the exception handler breaks. With the patch we trade that misleading message for improved DX.

I'm not totally sold on the idea that

Drupal should support custom exception handlers

That said I don't think we should get in the way as much as we do and we definitely should let our exception handler do it's job without calling function marked as private directly from DrupalKernel.

For me the only remaining question is should we be catching container errors that might be fixed by a rebuilding and adding info to the message? In the issue summary of #2509898: Additional uncaught exception thrown while handling exception after service changes it says

This means that DrupalKernel::handleException() is not correctly getting to the message that sends people to rebuild.php

which implies that that issue actually fixed it so that people now do see the rebuild message and it is helpful. However looking at the before and after screenshots in the issue summary it's obvious that @znerol is correct and the rebuild message is not displayed anymore.

Do we ever want to direct people to rebuild.php and if so, when?

Filed #2548823: Display an error message if the container needs to be rebuilt. I guess this issue here is fixed?

Lol. Okay I committed this accidentally (sorry everyone) as I wanted

Do we ever want to direct people to rebuild.php and if so, when?

to be discussed here.

However as @znerol opened #2548823: Display an error message if the container needs to be rebuilt I think we can just have that discussion there.

This broke the functionality that we had (I added, so biased :)) to NOT return 200 errors for uncaught exceptions. So seems like a regression. E.g. a fatal error would now return you a 200 response again. Unless I missed something, of course.

Nevermind, I think it might just be me and this works great.

I'm wondering what the status of this issue is. It is marked as closed / fixed. But as far as I understand this hasn't been implemented in Core.

I'm running into a situation where this might be a solution for my problem. Explained here: https://drupal.stackexchange.com/questions/288697/handle-nested-exceptio...

The fact that the default Drupal exception handler doesn't rethrow a catched exception is causing some headache.

Hi,
Is there any alternative way to handle custom exceptions for database settings.