Add a stacktrace to the log messagen when `$app['debug'] = true;` #613

Closed
wants to merge 3 commits into
from

Projects

None yet

7 participants

@till
till commented Feb 4, 2013

No description provided.

@davedevelopment

I'd be 👎 on this, I'd rather have this as an extra switch.

We could probably pass the exception ala PSR-3?

@till
till commented Feb 4, 2013

What extra switch do you mean? A configuration item like $app['monolog.stacktrace'] ? I am fine with either, I just want the stacktraces.

@igorw
igorw commented Feb 4, 2013

Generally 👍

But I agree that it should be a separate option. Nitpick: no need for true ===, since it's a boolean anyway.

@davedevelopment

@till yes, something that I can switch on or off.

@till
till commented Feb 5, 2013

@davedevelopment, @igorw — Let me know what you think.

@davedevelopment

To my knowledge the exception needs to be passed with a key of exception .e.g

$app['monolog']->addCritical($message, array('exception' => $e));
@fabpot
Member
fabpot commented Mar 8, 2013

I think @davedevelopment suggestion is all we need to do. It's then up to Monolog to deal with the exception as this is now a special key in PSR-3. ping @Seldaek

@Seldaek
Seldaek commented Mar 8, 2013

You should definitely do what @davedevelopment said, but while at the moment monolog dumps a whole stack trace, I'm not sure that's the best option for the LineFormatter at least. See symfony/symfony#7259 as well.

@fabpot
Member
fabpot commented Mar 8, 2013

@Seldaek: Anything planned for Monolog in the near future? I mean, we need to "fix" the Symfony issue anyway.

@Seldaek
Seldaek commented Mar 8, 2013

Fixing the line formatter shouldn't be too hard, just need to serialize the exceptions by hand. I should have some more time this weekend or next week latest.

@fabpot
Member
fabpot commented Apr 1, 2013

The exception issue has been fixed in Monolog now (see Seldaek/monolog#eaf2b07); we just have to wait for 1.4.1 to be released.

@Seldaek
Seldaek commented Apr 1, 2013

@fabpot could you confirm whether it really fixed it for usage in Silex/Symfony? I did fix something related to exceptions, but I couldn't completely reproduce the issue in a test so not sure if it is fixed or not.

@fabpot
Member
fabpot commented Apr 1, 2013

I don't see the issue myself, so we need to wait for someone else to confirm.

@Seldaek
Seldaek commented Apr 1, 2013

Alright, well I'll try to push out a release then and we see what happens.

@Seldaek
Seldaek commented Apr 1, 2013

Monolog 1.4.1 is out, for anyone that can try and reproduce this.

@gunnarlium gunnarlium commented on the diff Apr 7, 2013
src/Silex/Provider/MonologServiceProvider.php
@@ -72,10 +72,13 @@ public function boot(Application $app)
$app->error(function (\Exception $e) use ($app) {
$message = sprintf('%s: %s (uncaught exception) at %s line %s', get_class($e), $e->getMessage(), $e->getFile(), $e->getLine());
+ if ($app['monolog.stacktrace']) {
@gunnarlium
gunnarlium Apr 7, 2013

I believe you should add $app->offsetExists('monolog.stacktrace') or preferably provide a default value in the MonologServiceProvider.

@stof
stof commented Apr 7, 2013

I vote for closing this as Monolog handles the exception properly now, so following the PSR-3 advice is a better solution

@till till closed this Apr 7, 2013
@fabpot
Member
fabpot commented Apr 8, 2013

I've just created a quick PR for PSR3 in fabpot/Silex#670

@fabpot fabpot added a commit that referenced this pull request Apr 8, 2013
@fabpot fabpot merged branch fabpot/monolog-exceptions (PR #670)
This PR was merged into the master branch.

Discussion
----------

added exception in the Monolog context when appropriate (closes #613, closes #614)

Added support for PSR-3 logging and the special exception key.

Commits
-------

973ee70 added exception in the Monolog context when appropriate (closes #613, closes #614)
80f0abd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment