rollbar / rollbar-php-laravel

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

Fatal error after upgrading to 7.2.0 #134

Closed matt-h closed 2 years ago

matt-h commented 2 years ago

I'm getting the following error every time it tries to log anything to Rollbar. This is using Laravel 8 with the following config.

'rollbar' => [
            'driver' => 'monolog',
            'handler' => \Rollbar\Laravel\MonologHandler::class,
            'access_token' => env('ROLLBAR_TOKEN'),
            'level' => 'debug',
            'person_fn' => 'Auth::user',
            'capture_email' => true,   // optional
            'capture_username' => true, // optional
],
production.ERROR: Uncaught TypeError: Rollbar\Payload\Person::__construct(): Argument #1 ($id) must be of type string, int given, called in /srv/http/vendor/rollbar/rollbar/src/DataBuilder.php on line 916 and defined in /srv/http/vendor/rollbar/rollbar/src/Payload/Person.php:18
Stack trace:
#0 /srv/http/vendor/rollbar/rollbar/src/DataBuilder.php(916): Rollbar\Payload\Person->__construct(1, '...', '...', Array)
#1 /srv/http/vendor/rollbar/rollbar/src/DataBuilder.php(384): Rollbar\DataBuilder->getPerson()
#2 /srv/http/vendor/rollbar/rollbar/src/Config.php(725): Rollbar\DataBuilder->makeData('...', Object(TypeError), Array)
#3 /srv/http/vendor/rollbar/rollbar/src/RollbarLogger.php(215): Rollbar\Config->getRollbarData('...', Object(TypeError), Array)
#4 /srv/http/vendor/rollbar/rollbar/src/RollbarLogger.php(120): Rollbar\RollbarLogger->getPayload('...', '...', Object(TypeError), Array)
#5 /srv/http/vendor/rollbar/rollbar/src/Handlers/ExceptionHandler.php(28): Rollbar\RollbarLogger->log('...', Object(TypeError), Array)
#6 [internal function]: Rollbar\Handlers\ExceptionHandler->handle(Object(TypeError))
#7 {main}
  thrown {"userId":1,"exception":"[object] (Symfony\\Component\\ErrorHandler\\Error\\FatalError(code: 0): Uncaught TypeError: Rollbar\\Payload\\Person::__construct(): Argument #1 ($id) must be of type string, int given, called in /srv/http/vendor/rollbar/rollbar/src/DataBuilder.php on line 916 and defined in /srv/http/vendor/rollbar/rollbar/src/Payload/Person.php:18
Stack trace:
#0 /srv/http/vendor/rollbar/rollbar/src/DataBuilder.php(916): Rollbar\\Payload\\Person->__construct(1, '...', '...', Array)
#1 /srv/http/vendor/rollbar/rollbar/src/DataBuilder.php(384): Rollbar\\DataBuilder->getPerson()
#2 /srv/http/vendor/rollbar/rollbar/src/Config.php(725): Rollbar\\DataBuilder->makeData('...', Object(TypeError), Array)
#3 /srv/http/vendor/rollbar/rollbar/src/RollbarLogger.php(215): Rollbar\\Config->getRollbarData('...', Object(TypeError), Array)
#4 /srv/http/vendor/rollbar/rollbar/src/RollbarLogger.php(120): Rollbar\\RollbarLogger->getPayload('...', '...', Object(TypeError), Array)
#5 /srv/http/vendor/rollbar/rollbar/src/Handlers/ExceptionHandler.php(28): Rollbar\\RollbarLogger->log('...', Object(TypeError), Array)
#6 [internal function]: Rollbar\\Handlers\\ExceptionHandler->handle(Object(TypeError))
#7 {main}
  thrown at /srv/http/vendor/rollbar/rollbar/src/Payload/Person.php:18)
[stacktrace]
#0 {main}
"}
cyrusradfar commented 2 years ago

👋🏽 we will follow up, @matt-h -- thanks for reporting.

danielmorell commented 2 years ago

Hi @matt-h, thank you for opening this issue!

In rollbar/rollbar v3.0.0 we added strong typing to this property as a string. This may have been a mistake. It should probably be a union type string | int. It seems pretty reasonable that the ID would be an int for relational DBs and a string for document DBs.

If we were not using constructor property promotion, the constructor could accept and string | int and then cast an int to a string. This would allow us to strongly type the property as a string. It looks like it should be a string in the API docs.

I feel like this would be the simplest solution. Are there any there primitive types we should natively support other than string and int that we reasonably believe people could use as IDs?

matt-h commented 2 years ago

I can't think of anything besides string or int that would be used as an ID.

It might make sense to keep the ID in src/Payload/Person.php as a string.

And then here: https://github.com/rollbar/rollbar-php/blob/4cf5c77c7b9be5e7fb3223361faa7fa2442c6cbd/src/DataBuilder.php#L916 Cast it to a string.

danielmorell commented 2 years ago

Casting to a string in the DataBuilder class is great idea. I double checked to make sure it is the only place we instantiate a Person instance. It is the only place. I have created a PR https://github.com/rollbar/rollbar-php/pull/562 with the solution.

Thank you @matt-h for your help on this!

danielmorell commented 2 years ago

I have released v3.1.2 which should resolve the issue.

chrillep commented 2 years ago

@danielmorell have you bumped rollbar-laravel aswell ?

danielmorell commented 2 years ago

Hi @chrillep, No. I have not. There should be no need. You can simply run the following terminal command in your project directory.

$ composer update rollbar/rollbar

It should update rollbar/rollbar to v3.1.2.

chrillep commented 2 years ago

Hi @chrillep, No. I have not. There should be no need. You can simply run the following terminal command in your project directory.

$ composer update rollbar/rollbar

It should update rollbar/rollbar to v3.1.2.

@danielmorell yep that worked! cheers! but dependabot didn't pick it up fyi :)