fix: harden Messenger validation and review feedback

Validate Messenger adapter arrays at runtime so invalid elements fail with a clear InvalidArgumentException instead of a PHP error. This keeps the new Adapter|Adapter[] constructor ergonomic without weakening input validation.

Also replace Messenger sprintf-based error construction with direct string concatenation, pluralize the single-adapter failure message correctly, and add test coverage for single-adapter construction and invalid array elements.

Author: TorstenDittmann

src/Utopia/Messaging/Messenger.php

@@ -37,7 +37,7 @@ class Messenger
      *                                            At least one adapter must be provided.
      *                                            All adapters must support the same message type.
      *
-     * @throws \InvalidArgumentException If no adapters are provided or adapters have mixed types.
+     * @throws \InvalidArgumentException If no adapters are provided, an array element is not an adapter, or adapters have mixed types.
      */
     public function __construct(Adapter|array $adapters)
     {
@@ -49,6 +49,18 @@ public function __construct(Adapter|array $adapters)
             throw new \InvalidArgumentException('At least one adapter must be provided.');
         }
 
+        foreach ($adapters as $index => $adapter) {
+            if (! $adapter instanceof Adapter) {
+                throw new \InvalidArgumentException(
+                    'All elements must be instances of Adapter, but element '
+                    .$index
+                    .' is '
+                    .\get_debug_type($adapter)
+                    .'.'
+                );
+            }
+        }
+
         $this->validateAdapters($adapters);
 
         $this->adapters = $adapters;
@@ -76,31 +88,38 @@ public function send(Message $message): array
         $messageType = $this->adapters[0]->getMessageType();
 
         if (! \is_a($message, $messageType)) {
-            throw new \Exception(sprintf(
-                'Invalid message type. Expected "%s", got "%s".',
-                $messageType,
-                get_class($message)
-            ));
+            throw new \Exception(
+                'Invalid message type. Expected "'
+                .$messageType
+                .'", got "'
+                .\get_class($message)
+                .'".'
+            );
         }
 
         foreach ($this->adapters as $index => $adapter) {
             try {
                 return $adapter->send($message);
             } catch (\Exception $e) {
-                $errors[] = sprintf(
-                    '%s (adapter %d): %s',
-                    $adapter->getName(),
-                    $index + 1,
-                    $e->getMessage()
-                );
+                $errors[] = $adapter->getName()
+                    .' (adapter '
+                    .($index + 1)
+                    .'): '
+                    .$e->getMessage();
             }
         }
 
-        throw new \Exception(sprintf(
-            "All %d adapters failed:\n%s",
-            count($this->adapters),
-            implode("\n", $errors)
-        ));
+        $adapterCount = \count($this->adapters);
+        $adapterLabel = $adapterCount === 1 ? 'adapter' : 'adapters';
+
+        throw new \Exception(
+            'All '
+            .$adapterCount
+            .' '
+            .$adapterLabel
+            ." failed:\n"
+            .\implode("\n", $errors)
+        );
     }
 
     /**
@@ -151,25 +170,33 @@ private function validateAdapters(array $adapters): void
         $expectedType = $firstAdapter->getType();
         $expectedMessageType = $firstAdapter->getMessageType();
 
-        foreach ($adapters as $index => $adapter) {
+        foreach (\array_slice($adapters, 1, preserve_keys: true) as $index => $adapter) {
             if ($adapter->getType() !== $expectedType) {
-                throw new \InvalidArgumentException(sprintf(
-                    'All adapters must be of the same type. Expected "%s", but adapter %d (%s) has type "%s".',
-                    $expectedType,
-                    $index + 1,
-                    $adapter->getName(),
-                    $adapter->getType()
-                ));
+                throw new \InvalidArgumentException(
+                    'All adapters must be of the same type. Expected "'
+                    .$expectedType
+                    .'", but adapter '
+                    .($index + 1)
+                    .' ('
+                    .$adapter->getName()
+                    .') has type "'
+                    .$adapter->getType()
+                    .'".'
+                );
             }
 
             if ($adapter->getMessageType() !== $expectedMessageType) {
-                throw new \InvalidArgumentException(sprintf(
-                    'All adapters must support the same message type. Expected "%s", but adapter %d (%s) supports "%s".',
-                    $expectedMessageType,
-                    $index + 1,
-                    $adapter->getName(),
-                    $adapter->getMessageType()
-                ));
+                throw new \InvalidArgumentException(
+                    'All adapters must support the same message type. Expected "'
+                    .$expectedMessageType
+                    .'", but adapter '
+                    .($index + 1)
+                    .' ('
+                    .$adapter->getName()
+                    .') supports "'
+                    .$adapter->getMessageType()
+                    .'".'
+                );
             }
         }
     }

tests/Messaging/MessengerTest.php

@@ -164,12 +164,36 @@ public function test_throws_when_single_adapter_fails(): void
         );
 
         $this->expectException(\Exception::class);
-        $this->expectExceptionMessage('All 1 adapters failed');
+        $this->expectExceptionMessage('All 1 adapter failed');
         $this->expectExceptionMessage('OnlyAdapter (adapter 1): Network error');
 
         $messenger->send($message);
     }
 
+    public function test_accepts_single_adapter_instance(): void
+    {
+        $adapter = $this->createMock(Adapter::class);
+        $adapter->method('getName')->willReturn('OnlyAdapter');
+        $adapter->method('getType')->willReturn('sms');
+        $adapter->method('getMessageType')->willReturn(SMS::class);
+        $adapter->method('getMaxMessagesPerRequest')->willReturn(100);
+        $adapter->method('send')->willReturn([
+            'deliveredTo' => 1,
+            'type' => 'sms',
+            'results' => [['recipient' => '+1234567890', 'status' => 'success', 'error' => '']],
+        ]);
+
+        $messenger = new Messenger($adapter);
+
+        $result = $messenger->send(new SMS(
+            to: ['+1234567890'],
+            content: 'Test message'
+        ));
+
+        $this->assertEquals(1, $result['deliveredTo']);
+        $this->assertEquals('success', $result['results'][0]['status']);
+    }
+
     public function test_rejects_empty_adapter_list(): void
     {
         $this->expectException(\InvalidArgumentException::class);
@@ -178,6 +202,17 @@ public function test_rejects_empty_adapter_list(): void
         new Messenger([]);
     }
 
+    public function test_rejects_non_adapter_array_element(): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('All elements must be instances of Adapter, but element 1 is string.');
+
+        new Messenger([
+            $this->createMock(Adapter::class),
+            'not-an-adapter',
+        ]);
+    }
+
     public function test_rejects_mixed_adapter_types(): void
     {
         $smsAdapter = $this->createMock(Adapter::class);