rollbar / rollbar-php-laravel

Rollbar error monitoring integration for Laravel projects
https://docs.rollbar.com/docs/laravel
140 stars 39 forks source link

Person TypeError #73

Closed TylerVigario closed 5 years ago

TylerVigario commented 6 years ago

AFAIK, Laravel returns models in an object format and Rollbar-PHP-Laravel doesn't seem to handle this properly.

Using this from the docs: 'person_fn' => 'Auth::user'

Gives this error: Argument 4 passed to Rollbar\Payload\Person::__construct() must be of the type array or null, object given, called in C:\inetpub\wwwroot\oim-laravel\vendor\rollbar\rollbar\src\DataBuilder.php on line 894

Fixed by extending MonologHandler.php line 47 to serialize object to array: $person = call_user_func($config['person_fn'])->toArray();

The only reason I didn't submit a PR is that I don't like my solution as it expects to receive a Laravel model in return. I could add some elaborate type checking but wanted to see if anyone else is having this issue (albeit no results on issue tracker) and how they solved it.

I also tried:

'person_fn' => function () {
    $user = \Auth::user();

    return ($user === null ? null : [
        'id' => $user->id,
        'username' => $user->username,
        'email' => $user->email
    ]);
}

However, Laravel complained about closures when compiling cache.

jessewgibbs commented 6 years ago

@MeekLogic thanks for bringing this to our attention.

@ArturMoczulski can you take a look at this?

TylerVigario commented 5 years ago

I have created a PR for this – #75

TimothyDLewis commented 4 years ago

I realize this is closed, but just ran into an interesting caveat related to this error. Figured I'd share this information in case anyone else runs into the same issue.

My configuration is as follows:

'rollbar' => [
  'driver' => 'monolog',
  'handler' => \Rollbar\Laravel\MonologHandler::class,
  'access_token' => env('ROLLBAR_TOKEN'),
  'level' => 'info', // https://laravel.com/docs/8.x/logging#writing-log-messages
  'person_fn' => 'Auth::user'
],

And by all accounts, was functioning just fine. But, my User.php model does not use email, but rather email_address. In MonologHandler.php there's this line:

if (isset($data->email)) {
  $person['email'] = $data->email;
}

By default, that shouldn't do anything innocuous, as $data->email should be null. Unfortunately, my User.php model also had an unused (carry-over) method:

public function email() {
  return $this->email_address;
}

When called as $data->email() (function/method), this properly returns the email_address as a string. When called in the check via $data->email (property), this actually throws an exception:

App\Models\User::email must return a relationship instance.

Behind the scenes, Laravel has logic to translate methods to properties, which is why both $model->relationship and $model->relationship() ... works. To get around this issue, I properly used an accessor:

public function getEmailAttribute() {
  return $this->email_address;
}

Now, the check $data->email, as an accessor, properly returns the email_address as a string, and Rollbar can continue properly reporting.