feat: add AfricasTalking SMS adapter#132
Conversation
deepshekhardas
commented
Jun 17, 2026
- New SMS adapter for AfricasTalking API
- Supports sending SMS messages with optional 'from' field
- Handles up to 100 messages per request
- Proper error handling for API responses
- Includes integration tests
Greptile SummaryThis PR adds an AfricasTalking SMS adapter with support for an optional sender
Confidence Score: 3/5The adapter silently swallows all API error details on every failed send because the JSON response format is never requested from Africa's Talking. Without Files Needing Attention: src/Utopia/Messaging/Adapter/SMS/AfricasTalking.php — the request headers block needs Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/Utopia/Messaging/Adapter/SMS/AfricasTalking.php:45-48
**Missing `Accept: application/json` header causes silent error swallowing**
Africa's Talking returns XML by default; JSON is only returned when the request includes `Accept: application/json`. The base `buildResult()` method tries `json_decode()` on the body and silently ignores a `JsonException`, leaving `$result['response']` as a raw XML string. In the success path this doesn't matter, but in the error path both `isset($result['response']['errorMessage'])` and `isset($result['response']['message'])` will be false on a string value, so every API-level error will report "Unknown error" regardless of what Africa's Talking actually returned.
```suggestion
$result = $this->request(
method: 'POST',
url: self::API_URL,
headers: [
'Content-Type: application/x-www-form-urlencoded',
'Accept: application/json',
'apiKey: ' . $this->apiKey,
],
```
Reviews (6): Last reviewed commit: "fix: correct API endpoint and preserve p..." | Re-trigger Greptile |
| if ($result['statusCode'] === 201) { | ||
| $response->incrementDeliveredTo(); | ||
| $response->addResult($recipient); |
There was a problem hiding this comment.
Africa's Talking returns per-recipient statuses in SMSMessageData.Recipients, including failures such as invalid numbers or insufficient balance, while the HTTP request itself can still succeed. This branch marks every 201 response as delivered without reading those recipient statuses, so callers can receive success and an incremented deliveredTo count for messages the provider rejected.
Artifacts
Repro: focused PHP harness mocking a 201 failed-recipient Africa's Talking response
- Contains supporting evidence from the run (text/x-php; charset=utf-8).
Repro: harness execution output showing HTTP 201 Created and adapter success/deliveredTo behavior
- Keeps the command output available without making the summary code-heavy.
| public function getMaxMessagesPerRequest(): int | ||
| { | ||
| return 100; | ||
| } |
There was a problem hiding this comment.
getMaxMessagesPerRequest claims 100 but makes one HTTP call per recipient
getMaxMessagesPerRequest() returning 100 tells the base Adapter::send() to allow up to 100 recipients in a single call, but process() loops over each one and fires a separate $this->request() for every number. A caller passing 100 recipients will trigger 100 sequential HTTP round-trips rather than the single batched request the value implies, and can easily hit Africa's Talking rate limits or exhaust the 30-second socket timeout.
The Africa's Talking API supports multiple recipients natively — the to parameter accepts a comma-separated list in one POST. The adapter should either join all recipients into a single request body (and process the per-recipient statuses from SMSMessageData.Recipients) or, as a minimal safe fix, change getMaxMessagesPerRequest() to 1 to match the actual one-request-per-recipient behavior, consistent with how Twilio and Vonage adapters are implemented.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Utopia/Messaging/Adapter/SMS/AfricasTalking.php
Line: 28-31
Comment:
**`getMaxMessagesPerRequest` claims 100 but makes one HTTP call per recipient**
`getMaxMessagesPerRequest()` returning `100` tells the base `Adapter::send()` to allow up to 100 recipients in a single call, but `process()` loops over each one and fires a separate `$this->request()` for every number. A caller passing 100 recipients will trigger 100 sequential HTTP round-trips rather than the single batched request the value implies, and can easily hit Africa's Talking rate limits or exhaust the 30-second socket timeout.
The Africa's Talking API supports multiple recipients natively — the `to` parameter accepts a comma-separated list in one POST. The adapter should either join all recipients into a single request body (and process the per-recipient statuses from `SMSMessageData.Recipients`) or, as a minimal safe fix, change `getMaxMessagesPerRequest()` to `1` to match the actual one-request-per-recipient behavior, consistent with how Twilio and Vonage adapters are implemented.
How can I resolve this? If you propose a fix, please make it concise.
|
Following up - AfricasTalking SMS adapter. Ready for review. |
- New SMS adapter for AfricasTalking API - Supports sending SMS messages with optional 'from' field - Handles up to 100 messages per request - Proper error handling for API responses - Includes integration tests
…decode to assertResponse
| headers: [ | ||
| 'Content-Type: application/x-www-form-urlencoded', | ||
| 'apiKey: ' . $this->apiKey, | ||
| ], |
There was a problem hiding this comment.
Accept: application/json header causes silent error swallowing
Africa's Talking returns XML by default; JSON is only returned when the request includes Accept: application/json. The base buildResult() method tries json_decode() on the body and silently ignores a JsonException, leaving $result['response'] as a raw XML string. In the success path this doesn't matter, but in the error path both isset($result['response']['errorMessage']) and isset($result['response']['message']) will be false on a string value, so every API-level error will report "Unknown error" regardless of what Africa's Talking actually returned.
| headers: [ | |
| 'Content-Type: application/x-www-form-urlencoded', | |
| 'apiKey: ' . $this->apiKey, | |
| ], | |
| $result = $this->request( | |
| method: 'POST', | |
| url: self::API_URL, | |
| headers: [ | |
| 'Content-Type: application/x-www-form-urlencoded', | |
| 'Accept: application/json', | |
| 'apiKey: ' . $this->apiKey, | |
| ], |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Utopia/Messaging/Adapter/SMS/AfricasTalking.php
Line: 45-48
Comment:
**Missing `Accept: application/json` header causes silent error swallowing**
Africa's Talking returns XML by default; JSON is only returned when the request includes `Accept: application/json`. The base `buildResult()` method tries `json_decode()` on the body and silently ignores a `JsonException`, leaving `$result['response']` as a raw XML string. In the success path this doesn't matter, but in the error path both `isset($result['response']['errorMessage'])` and `isset($result['response']['message'])` will be false on a string value, so every API-level error will report "Unknown error" regardless of what Africa's Talking actually returned.
```suggestion
$result = $this->request(
method: 'POST',
url: self::API_URL,
headers: [
'Content-Type: application/x-www-form-urlencoded',
'Accept: application/json',
'apiKey: ' . $this->apiKey,
],
```
How can I resolve this? If you propose a fix, please make it concise.