laravel-rollbar icon indicating copy to clipboard operation
laravel-rollbar copied to clipboard

Duplicate

Open jesseleite opened this issue 8 years ago • 7 comments

Your usage documentation suggests using Log facade to log all exceptions for Rollbar. ie)

public function report(Exception $e)
{
    \Log::error($e);
    return parent::report($e);
}

However, the parent::report($e) call there also logs exceptions. This means that reportable exceptions are being logged twice. Can we not just let the parent::report($e) method do the logging? Thoughts?

jesseleite avatar Jun 30 '16 20:06 jesseleite

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 avatar Jul 20 '16 10:07 gregrobson

@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...

gregory-claeyssens avatar Feb 03 '17 10:02 gregory-claeyssens

@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 avatar Feb 03 '17 16:02 jesseleite

@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;
}

gregory-claeyssens avatar Feb 03 '17 16:02 gregory-claeyssens

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?

jesseleite avatar Feb 03 '17 16:02 jesseleite

@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.

briandotdev avatar Mar 19 '17 15:03 briandotdev

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,
                ],
            ]);
        }

patrick-proulx avatar May 25 '18 15:05 patrick-proulx