[5.5] Exception Handler Improvement in report()

[ralphschindler]

Several monolog handlers (and logging services) have first class support for logging exceptions. (New relic for example: https://github.com/Seldaek/monolog/blob/master/src/Monolog/Handler/NewRelicHandler.php#L88) They are all consistent in that they expect the exception in the context at the key 'exception'. Additionally, it would be more user friendly to not have an exception (along with the whole large stacktrace) stringified in the message.

Tests are included, I am not sure what other expectations there are for logging exception in the laravel skeleton and other products. Thanks.

[taylorotwell]

I feel like this would be a big change for a lot of people who just use the default log files to look at stack traces, etc, which would no longer be available.

[ralphschindler]

You may be right, perhaps there is a middle ground I can find...

Would you be interested in this behavior being configurable/opt-in?

Another Thought: what if the first parameter was not $e->getMessage() - just $e as before, but it still passes the exception as a context value (that way a user log processor could affect the output to some degree if so desired, like for example cleaning up the message of the stack trace).

The unfortunate nature of Monolog is that processors happen after (string) $message happens here (in the 1.x series): https://github.com/Seldaek/monolog/blob/1.x/src/Monolog/Logger.php#L322-L323 , so its not possible to alter the logger with a monolog processor

As it happens, (and this is not meant to be critical in the least of the current strategy) Monolog really wants that $message to be a string at call time (not an object to be coerced, hence declare(strict_types=1);). I believe once you make the decision to move towards monolog-next version, you'll either have to stringify the message yourself in Illuminate's Error Handler report() method, or perhaps move to my suggested PR above. This is the method signature for monolog-next: https://github.com/Seldaek/monolog/blob/master/src/Monolog/Logger.php#L280

(edit: you can still expect Exceptions to be coerced even though the library uses declare(strict_types=1); in monolog-next, I mostly mean the intent conveyed by monolog is that they want a string there.)

[ralphschindler]

Something else to consider- monolog does have this as a configurable thing in the LineFormatter (see includeStacktraces related functionality:
https://github.com/Seldaek/monolog/blob/1.x/src/Monolog/Formatter/LineFormatter.php#L29

[taylorotwell]

Another option may be to have a new protected method log(Logger $logger, $e) method on the exception handler which just contains that $logger->error() line. That way it could be customized a bit easier maybe? Just thinking out loud.

[ralphschindler]

The most recent commit demonstrates a potential strategy that meets the goal of having the stacktraces in the new laravel/laravel project's default log files. Attached is a screenshot of what you can expect (this triggered from a new RuntimeException('controller exception') in the web.php file

As a sidenote, I am less interested in the protected method route. Our workflow for new in-house apps is such that we start with a laravel new project, then we add in our in-house packages/service-providers to customize the default app with company specific "things".. (one of those things being what log services we use in a production environment). Put simply, we've chosen to focus on extending apps via a ServiceProvider's boot() or register() method, rather than altering /app code, at least as much as possible.

[taylorotwell]

Looks good, thanks.