Repository: rollbar/rollbar-php (php)

Issue:
New TelemetryEvent cannot handle nested array as $body argument
I'm using the newest release of the Laravel PHP package which uses the 4.1.0 version of this package. This version can result in the following error: 
```
Named parameter $message overwrites previous argument {"exception":"[object] (Error(code: 0): Named parameter $message overwrites previous argument at vendor/rollbar/rollbar/src/Payload/TelemetryEvent.php:50)
```

This error is thrown if the body argument of the TelemetryEvent __construct function is a nested array. For example (when using the Laravel-Rollbar package):
```
Log::info('This throws an error', [["data" => "some data]]);
```
The error is caused by the top-level nested array. This worked before, so this is regression of functionality.

**Simple replication**
To replicate you can do the following, which is simple direct replication of what happens in the constructor of the `TelemetryEvent` class.
```
// This throws an error
$event = new TelemetryBody(...[['data' => 'some_data']]);
```

One lever higher up, there is a restriction on what is allowed in the array of the body. The following will cause an error:
```
new TelemetryEvent(
      EventType::Log,
      EventLevel::Info,
      ["0" => ["id" => "some id"], 'message' => 'test message'],
 );
```

The above is a simplification of how the TelementryEvent is used within Rollbar-Laravel package: https://github.com/rollbar/rollbar-php-laravel/blob/f430c09887d3603bcefc870fd7a9e7abcd009920/src/TelemetryListener.php#L104 which triggers this issue.

Pull request:
Fixed #648 TelemetryEvent cannot handle nested array as $body argument.
## Description of the change

This resolves an issue with the current creation of a `TelemetryBody` instance in `TelemetryEvent`. The core of the issue surrounds how the PHP spread operator (`...`) works when it is uesd to expand an array into a function's named arguments.

If an array has both string and numeric keys it can cause issues. The numeric keys are passed the the function as positional arguments. While the string keys are passed to the function as named arguments. If both a numeric key and a string key refer to the same argument PHP will throw and exception.

Consider the following example...

```php
$array = [
    0 => 'foo',
    'message' => 'bar',
];

function foo(string $message, string $other) {
    echo $message;
}

foo(...$array);
```

In this example both the `0` and `'message'` keys resolve to the first argument, which causes PHP to throw an exception.

Because we have been using the spread operator when the body of `TelemetryEvent` is an array we are experiancing the same issue. I am fixing this issue in this PR by adding a new `fromArray()` method on `TelemetryBody`. The new method is then used instead of the constructor for array bodys.

## Type of change

- [x] Bug fix (non-breaking change that fixes an issue)

## Related issues

- Fix #648
- SDK-455

## 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/Payload/TelemetryBody.php b/src/Payload/TelemetryBody.php
index 3bb84a5..7fc1d35 100644
--- a/src/Payload/TelemetryBody.php
+++ b/src/Payload/TelemetryBody.php
@@ -70,6 +70,29 @@ class TelemetryBody implements SerializerInterface
         $this->extra = $extra;
     }
 
+    /**
+     * Creates a {@see TelemetryBody} instance from an array of data.
+     *
+     * The data array may be loosely structured, as only the keys that match the defined keys will be used to create the
+     * instance. Any undefined keys in data will be stored in the {@see $extra} property.
+     *
+     * @param array $data The data to create the {@see TelemetryBody} instance from.
+     * @return self
+     *
+     * @since 4.1.1
+     */
+    public static function fromArray(array $data): self
+    {
+        // This filters out any keys that are not accepted by the constructor to prevent duplicate parameter errors from
+        // named and positional arguments.
+        $params = array_intersect_key($data, array_flip(self::DEFINED_KEYS));
+        // Generates an array of all the keys not used in the constructor.
+        $extra = array_diff_key($data, $params);
+        $instance = new self(...$params);
+        $instance->extra = $extra;
+        return $instance;
+    }
+
     /**
      * Returns the array representation of the telemetry body.
      *
diff --git a/src/Payload/TelemetryEvent.php b/src/Payload/TelemetryEvent.php
index a3ce387..019f337 100644
--- a/src/Payload/TelemetryEvent.php
+++ b/src/Payload/TelemetryEvent.php
@@ -47,7 +47,7 @@ class TelemetryEvent implements SerializerInterface
         if (is_null($this->timestamp)) {
             $this->timestamp = floor(microtime(true) * 1000);
         }
-        $this->body = is_array($body) ? new TelemetryBody(...$body): $body;
+        $this->body = is_array($body) ? TelemetryBody::fromArray($body): $body;
     }
 
     public function serialize(): array


Test patch:
diff --git a/tests/Payload/TelemetryBodyTest.php b/tests/Payload/TelemetryBodyTest.php
index 3043455..1c3721a 100644
--- a/tests/Payload/TelemetryBodyTest.php
+++ b/tests/Payload/TelemetryBodyTest.php
@@ -97,4 +97,55 @@ class TelemetryBodyTest extends BaseRollbarTest
 
         self::assertSame(['message' => 'foo'], $body->serialize());
     }
+
+    /**
+     * @since 4.1.1
+     */
+    public function testFromArray(): void
+    {
+        $body = TelemetryBody::fromArray([
+            'message' => 'message',
+            'method' => 'method',
+            'url' => 'url',
+            'status_code' => 'status',
+            'subtype' => 'sub',
+            'stack' => 'stack',
+            'from' => 'from',
+            'to' => 'to',
+            'start_timestamp_ms' => 42,
+            'end_timestamp_ms' => 43,
+            'extraOne' => 'foo',
+            'extraTwo' => 'bar',
+        ]);
+
+        self::assertSame('message', $body->message);
+        self::assertSame('method', $body->method);
+        self::assertSame('url', $body->url);
+        self::assertSame('status', $body->status_code);
+        self::assertSame('sub', $body->subtype);
+        self::assertSame('stack', $body->stack);
+        self::assertSame('from', $body->from);
+        self::assertSame('to', $body->to);
+        self::assertSame(42, $body->start_timestamp_ms);
+        self::assertSame(43, $body->end_timestamp_ms);
+        self::assertSame([
+            'extraOne' => 'foo',
+            'extraTwo' => 'bar',
+        ], $body->extra);
+    }
+
+    /**
+     * @since 4.1.1
+     */
+    public function testFromArrayNested(): void
+    {
+        // Assert that nested/non-standard arrays don't throw an error.
+        $body = TelemetryBody::fromArray([["data" => "some data"]]);
+        self::assertSame([["data" => "some data"]], $body->extra);
+
+        // Assert that positional arguments are not passed to named arguments on the constructor.
+        $body = TelemetryBody::fromArray(["0" => ["id" => "some id"], 'message' => 'test message']);
+        self::assertSame('test message', $body->message);
+        self::assertSame(["0" => ["id" => "some id"]], $body->extra);
+    }
 }
diff --git a/tests/Payload/TelemetryEventTest.php b/tests/Payload/TelemetryEventTest.php
index ddf3d10..0beba13 100644
--- a/tests/Payload/TelemetryEventTest.php
+++ b/tests/Payload/TelemetryEventTest.php
@@ -20,4 +20,30 @@ class TelemetryEventTest extends BaseRollbarTest
         self::assertGreaterThanOrEqual($before, $event->timestamp);
         self::assertLessThanOrEqual($after, $event->timestamp);
     }
+
+    /**
+     * @since 4.1.1
+     */
+    public function testNestedArrayBody(): void
+    {
+        $event = new TelemetryEvent(EventType::Network, EventLevel::Info, [
+            'method' => 'GET',
+            'url' => 'https://example.com',
+            'status_code' => '200',
+            [
+                'unstructured' => 'data',
+                0 => 'foo',
+            ],
+        ]);
+
+        self::assertSame('GET', $event->body->method);
+        self::assertSame('https://example.com', $event->body->url);
+        self::assertSame('200', $event->body->status_code);
+        self::assertSame([
+            [
+                'unstructured' => 'data',
+                0 => 'foo',
+            ],
+        ], $event->body->extra);
+    }
 }