bug #42053 [Notifier] [Smsapi] fixed checking whether message is sent (damlox)

fabpot · fabpot · commit 1e91b1a3e5e0 · 2021-08-26T09:20:34.000+02:00
This PR was merged into the 5.3 branch. Discussion ---------- [Notifier] [Smsapi] fixed checking whether message is sent | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - SmsAPI always return 200 status code even if we enter incorrect token. The content is: `{"error":101,"message":"Authorization failed"}`. Commits ------- b663ea3 notifier smsapi - fixed checking whether message is sent
diff --git a/src/Symfony/Component/Notifier/Bridge/Smsapi/SmsapiTransport.php b/src/Symfony/Component/Notifier/Bridge/Smsapi/SmsapiTransport.php
@@ -18,6 +18,7 @@
 use Symfony\Component\Notifier\Message\SmsMessage;
 use Symfony\Component\Notifier\Transport\AbstractTransport;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
+use Symfony\Contracts\HttpClient\Exception\DecodingExceptionInterface;
 use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
@@ -72,10 +73,14 @@ protected function doSend(MessageInterface $message): SentMessage
             throw new TransportException('Could not reach the remote Smsapi server.', $response, 0, $e);
         }
 
-        if (200 !== $statusCode) {
-            $error = $response->toArray(false);
+        try {
+            $content = $response->toArray(false);
+        } catch (DecodingExceptionInterface $e) {
+            throw new TransportException('Could not decode body to an array.', $response, 0, $e);
+        }
 
-            throw new TransportException(sprintf('Unable to send the SMS: "%s".', $error['message']), $response);
+        if ((isset($content['error']) && null !== $content['error']) || (200 !== $statusCode)) {
+            throw new TransportException(sprintf('Unable to send the SMS: "%s".', $content['message'] ?? 'unknown error'), $response);
         }
 
         return new SentMessage($message, (string) $this);
diff --git a/src/Symfony/Component/Notifier/Bridge/Smsapi/Tests/SmsapiTransportTest.php b/src/Symfony/Component/Notifier/Bridge/Smsapi/Tests/SmsapiTransportTest.php
@@ -11,7 +11,10 @@
 
 namespace Symfony\Component\Notifier\Bridge\Smsapi\Tests;
 
+use Symfony\Component\HttpClient\MockHttpClient;
+use Symfony\Component\HttpClient\Response\MockResponse;
 use Symfony\Component\Notifier\Bridge\Smsapi\SmsapiTransport;
+use Symfony\Component\Notifier\Exception\TransportException;
 use Symfony\Component\Notifier\Message\ChatMessage;
 use Symfony\Component\Notifier\Message\MessageInterface;
 use Symfony\Component\Notifier\Message\SmsMessage;
@@ -44,4 +47,40 @@ public function unsupportedMessagesProvider(): iterable
         yield [new ChatMessage('Hello!')];
         yield [$this->createMock(MessageInterface::class)];
     }
+
+    public function createClient(int $statusCode, string $content): HttpClientInterface
+    {
+        return new MockHttpClient(new MockResponse($content, ['http_code' => $statusCode]));
+    }
+
+    public function responseProvider(): iterable
+    {
+        $responses = [
+            ['status' => 200, 'content' => '{"error":101,"message":"Authorization failed"}', 'errorMessage' => 'Unable to send the SMS: "Authorization failed".'],
+            ['status' => 500, 'content' => '{}', 'errorMessage' => 'Unable to send the SMS: "unknown error".'],
+            ['status' => 500, 'content' => '{"error":null,"message":"Unknown"}', 'errorMessage' => 'Unable to send the SMS: "Unknown".'],
+            ['status' => 500, 'content' => '{"error":null,"message":null}', 'errorMessage' => 'Unable to send the SMS: "unknown error".'],
+            ['status' => 500, 'content' => 'Internal error', 'errorMessage' => 'Could not decode body to an array.'],
+            ['status' => 200, 'content' => 'Internal error', 'errorMessage' => 'Could not decode body to an array.'],
+        ];
+
+        foreach ($responses as $response) {
+            yield [$response['status'], $response['content'], $response['errorMessage']];
+        }
+    }
+
+    /**
+     * @dataProvider responseProvider
+     */
+    public function testThrowExceptionWhenMessageWasNotSent(int $statusCode, string $content, string $errorMessage)
+    {
+        $client = $this->createClient($statusCode, $content);
+        $transport = $this->createTransport($client);
+        $message = new SmsMessage('0611223344', 'Hello!');
+
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage($errorMessage);
+
+        $transport->send($message);
+    }
 }
