Open jesseleite opened 8 years ago
This appears to be an issue with Laravel, having looked at. https://github.com/laravel/framework/blob/5.2/src/Illuminate/Foundation/Exceptions/Handler.php
parent::report(Exception $e)
is calling this parent method.
public function report(Exception $e)
{
if ($this->shouldReport($e)) {
$this->log->error($e);
}
}
$this->log
should be an implementation of LoggerInterface
and the signature should be error(Exception $e, array $context = [])
There's no way to pass contact through Laravel's implementation. I'll post up an issue later today. For the moment, it might be best to check whether Rollbar is enabled in your current environment and then decide whether to call \Log::error($e)
or parent::report($e)
@gregrobson It looks like Laravel does not see this an error from their side. How did you resolve this? Is it safe to drop \Log::error($e);
? I'm seeing 95% of exceptions as duplicates, but sometimes they still come as one... Which seems odd to me...
@sweet-greg the exceptions that only report once in rollbar are probably in your $dontReport
array in app/Hander.php
. The exceptions that report twice are not in your $dontReport
array, so they are being reported by the parent::report()
as well as your \Log::error($e)
call. That's my assumption anyway.
If that's the case, I think it would be much more intuitive to just let parent::report()
do it's thing, and not have laravel-rollbar users add the extra \Log::error($e)
. If I don't want to report a specific kind of exception, like a form ValidationException, then I likely don't want to see that exception in Rollbar. At my place of work, we just dropped the \Log::error($e)
from our app, and we see everything we need to see in Rollbar. If there's an exception we don't want to see, it goes in $dontReport
array 👍
TL;DR... I don't think there's any "fix" to be made here. Just stop suggesting \Log::error($e)
in the package README :) Thoughts?
@JesseLeite Thanks for getting back at me, if you check out parent::report()
the same check is done (although in reverse). So that should not be the reason for sometimes having no duplicates.
// Illuminate\Foundation\Exceptions\Handler@report
if ($this->shouldntReport($e)) {
return;
}
That's exactly what I was getting at, but in much less words 😃 parent::report()
respects your $dontReport
array, and I think Rollbar should follow suit and respect your $dontReport
array. The extra \Log::error($e)
is confusing things for users of this package, and causing the duplicates.
@jenssegers thoughts?
@gregrobson I agree with the issue you submitted to Laravel repo. It seems like @GrahamCampbell may have misunderstood what you were requesting. We want to pass the error context to parent::report($e)
and then we can remove \Log::error($e, $context)
entirely.
This is old, but I've found a solution if you're using the Rollbar Service Provider. In report(Exception $e)
, you can add:
if ($user = Auth::user()) {
app('Rollbar\RollbarLogger')->configure([
'person' => [
'id' => $user->id,
'username' => $user->first_name . ' ' . $user->last_name,
'email' => $user->email,
],
]);
}
Your usage documentation suggests using
Log
facade to log all exceptions for Rollbar. ie)However, the
parent::report($e)
call there also logs exceptions. This means that reportable exceptions are being logged twice. Can we not just let theparent::report($e)
method do the logging? Thoughts?