Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Person TypeError #73

Closed
TylerVigario opened this issue Nov 30, 2018 · 3 comments
Closed

Person TypeError #73

TylerVigario opened this issue Nov 30, 2018 · 3 comments
Assignees

Comments

@TylerVigario
Copy link

TylerVigario commented Nov 30, 2018

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.

@TylerVigario TylerVigario changed the title TypeError from person_fn => 'Auth::user' Person TypeError Nov 30, 2018
@jessewgibbs
Copy link

@MeekLogic thanks for bringing this to our attention.

@ArturMoczulski can you take a look at this?

@TylerVigario
Copy link
Author

I have created a PR for this – #75

@TimothyDLewis
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants