Repository: rollbar/rollbar-php (php)

Issue:
Rollbar\Payload\Person::__construct(): Argument #4 ($extra) must be of type array, App\Models\User given, called in /var/www/vendor/rollbar/rollbar/src/DataBuilder.php on line 949 and defined in /var/www/vendor/rollbar/rollbar/src/Payload/Person.php:18
**Description**:  
Hi, I’m experiencing an exception when Rollbar logs an error. My logger config is below:

```php
        'rollbar'           => [
            'driver'            => 'monolog',
            'handler'           => MonologHandler::class,
            'access_token'      => env('ROLLBAR_MONOLOG_TOKEN'),
            'level'             => 'debug',
            'person_fn'         => 'Auth::user',
            'capture_email'     => true,
            'capture_username'  => true,
            'processors'        => [
                ApiRequestProcessor::class,
            ],
        ],
```

and I think ```person_fn``` is contributing to this error:

```
Stack trace:
#0 /var/www/vendor/rollbar/rollbar/src/DataBuilder.php(949): Rollbar\Payload\Person->__construct('18', NULL, 'email@example.com', Object(App\Models\User))
rollbar/rollbar-php-laravel#1 /var/www/vendor/rollbar/rollbar/src/DataBuilder.php(380): Rollbar\DataBuilder->getPerson()
rollbar/rollbar-php-laravel#2 /var/www/vendor/rollbar/rollbar/src/Config.php(798): Rollbar\DataBuilder->makeData('error', Object(Error), Array)
rollbar/rollbar-php-laravel#3 /var/www/vendor/rollbar/rollbar/src/RollbarLogger.php(433): Rollbar\Config->getRollbarData('error', Object(Error), Array)
rollbar/rollbar-php-laravel#4 /var/www/vendor/rollbar/rollbar/src/RollbarLogger.php(256): Rollbar\RollbarLogger->getPayload('c04def903df748v4890e65135ff05500', 'error', Object(Error), Array)
rollbar/rollbar-php-laravel#5 /var/www/vendor/rollbar/rollbar/src/Handlers/ExceptionHandler.php(27): Rollbar\RollbarLogger->report('error', Object(Error), Array, true)
rollbar/rollbar-php-laravel#6 [internal function]: Rollbar\Handlers\ExceptionHandler->handle(Object(Error))
rollbar/rollbar-php-laravel#7 {main}
```

I use ```rollbar/rollbar-laravel``` v8.1.2.

Please let me know if I can help.



Pull request description:
Fixed #651 `Person::$extra` must be an array and an object was given.
## Description of the change

This resolves the issue with #651 by serializing the object passed to the extra parameter of `Person::__construct()`.

It also moves the logic to prevent using the keys `id`, `email`, and `username` from the `DataBuilder` class to the `Person` class as this seems like a more logical location. As in the `Person` constructor it can prevent these keys being set no mater where the instance is being created from.

## Type of change

- [x] Bug fix (non-breaking change that fixes an issue)

## Related issues

- SDK-466
- SDK-468
- Fix #651

## Checklists

### Development

- [x] Lint rules pass locally
- [x] The code changed/added as part of this pull request has been covered with tests
- [x] All tests related to the changed code pass in development

### Code review

- [ ] This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
- [ ] "Ready for review" label attached to the PR and reviewers assigned
- [ ] Issue from task tracker has a link to this pull request
- [ ] Changes have been reviewed by at least one other engineer


Patch:
diff --git a/src/DataBuilder.php b/src/DataBuilder.php
index 3495b17..ea1c414 100644
--- a/src/DataBuilder.php
+++ b/src/DataBuilder.php
@@ -945,6 +945,9 @@ class DataBuilder implements DataBuilderInterface
             $username = $personData['username'];
         }
 
+        if (!is_array($personData)) {
+            $personData = Utilities::serializeToArray($personData);
+        }
         unset($personData['id'], $personData['email'], $personData['username']);
         return new Person($identifier, $username, $email, $personData);
     }
diff --git a/src/Payload/Person.php b/src/Payload/Person.php
index 3c484bc..f8b143a 100644
--- a/src/Payload/Person.php
+++ b/src/Payload/Person.php
@@ -15,12 +15,21 @@ class Person implements SerializerInterface
 {
     use UtilitiesTrait;
 
+    /**
+     * Any additional data to be sent with the person object.
+     *
+     * @var array<string, mixed> $extra
+     */
+    private array $extra;
+
     public function __construct(
         private string $id,
         private ?string $username = null,
         private ?string $email = null,
-        private array $extra = []
+        array $extra = [],
     ) {
+        unset($extra['id'], $extra['email'], $extra['username']);
+        $this->extra = $extra;
     }
 
     public function getId(): string
diff --git a/src/Utilities.php b/src/Utilities.php
index 9fdc109..664edcb 100644
--- a/src/Utilities.php
+++ b/src/Utilities.php
@@ -2,6 +2,8 @@
 
 namespace Rollbar;
 
+use Serializable;
+
 final class Utilities
 {
     private static $ObjectHashes;
@@ -138,6 +140,36 @@ final class Utilities
 
         return $returnVal;
     }
+
+    /**
+     * Serialize the given object to an array.
+     *
+     * @param mixed $obj The object to serialize.
+     * @return array The serialized object as an array.
+     *
+     * @since 4.1.2
+     */
+    public static function serializeToArray(mixed $obj): array
+    {
+        if (is_array($obj)) {
+            return $obj;
+        }
+
+        // This is the most reliable way to serialize an object that does not potentially cause issues with other
+        // frameworks.
+        if ($obj instanceof Serializable && method_exists($obj, '__serialize')) {
+            return $obj->__serialize();
+        }
+
+        if (is_object($obj)) {
+            return array(
+                'type' => 'object',
+                'class' => get_class($obj),
+            );
+        }
+
+        return array('type' => gettype($obj));
+    }
     
     private static function serializeObject(
         $obj,
@@ -157,7 +189,7 @@ final class Utilities
         }
 
         // All other classes.
-        if ($obj instanceof \Serializable) {
+        if ($obj instanceof Serializable) {
             self::markSerialized($obj, $objectHashes);
             if (method_exists($obj, '__serialize')) {
                 return $obj->__serialize();


Test patch:
diff --git a/tests/PersonTest.php b/tests/PersonTest.php
index 98078bd..2de7e10 100644
--- a/tests/PersonTest.php
+++ b/tests/PersonTest.php
@@ -42,6 +42,13 @@ class PersonTest extends BaseRollbarTest
         $this->assertEquals("testing", $person->test);
     }
 
+    public function testExtraId(): void
+    {
+        $person = new Person('42', extra: ['id' => 'testing']);
+        $this->assertEquals('42', $person->getId());
+        $this->assertEquals(array('id' => '42'), $person->serialize());
+    }
+
     public function testEncode(): void
     {
         $person = new Person("1024");
diff --git a/tests/UtilitiesTest.php b/tests/UtilitiesTest.php
index 2be2f07..be1c4b3 100644
--- a/tests/UtilitiesTest.php
+++ b/tests/UtilitiesTest.php
@@ -1,6 +1,7 @@
 <?php namespace Rollbar;
 
 use Rollbar\Payload\Level;
+use Rollbar\TestHelpers\ArrayLogger;
 use Rollbar\TestHelpers\CustomSerializable;
 use Rollbar\TestHelpers\CycleCheck\ParentCycleCheck;
 use Rollbar\TestHelpers\CycleCheck\ChildCycleCheck;
@@ -113,6 +114,24 @@ class UtilitiesTest extends BaseRollbarTest
         $this->assertArrayNotHasKey("myNullValue", $result);
         $this->assertArrayNotHasKey("my_null_value", $result);
     }
+
+    public function testSerializeToArray(): void
+    {
+        $this->assertSame(array('type' => 'NULL'), Utilities::serializeToArray(null));
+        $this->assertSame(array('type' => 'integer'), Utilities::serializeToArray(4));
+        $this->assertSame(array(1, 2), Utilities::serializeToArray(array(1, 2)));
+        $this->assertSame(
+            array('foo' => 'bar'),
+            Utilities::serializeToArray(new CustomSerializable(array('foo' => 'bar'))),
+        );
+        $this->assertSame(
+            array(
+                'type' => 'object',
+                'class' => 'Rollbar\TestHelpers\ArrayLogger'
+            ),
+            Utilities::serializeToArray(new ArrayLogger()),
+        );
+    }
     
     public function testSerializationCycleChecking(): void
     {
@@ -131,17 +150,17 @@ class UtilitiesTest extends BaseRollbarTest
         
         $this->assertMatchesRegularExpression(
             '/<CircularReference.*/',
-            $result["obj"]["value"]["child"]["value"]["parent"]
+            $result["obj"]["value"]["child"]["value"]["parent"],
         );
         
         $this->assertMatchesRegularExpression(
             '/<CircularReference.*/',
-            $result["serializedObj"]["child"]["parent"]
+            $result["serializedObj"]["child"]["parent"],
         );
         
         $this->assertMatchesRegularExpression(
             '/<CircularReference.*/',
-            $result["payload"]["data"]["body"]["extra"][0]["value"]["child"]["value"]["parent"]
+            $result["payload"]["data"]["body"]["extra"][0]["value"]["child"]["value"]["parent"],
         );
     }