Skip to content

(feat): surface transport error details in FCM push results#136

Merged
ChiragAgg5k merged 1 commit into
mainfrom
feat/fcm-error-details
Jul 14, 2026
Merged

(feat): surface transport error details in FCM push results#136
ChiragAgg5k merged 1 commit into
mainfrom
feat/fcm-error-details

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

What

Improves FCM push delivery error reporting. Previously, when a request failed at the transport layer (timeout, DNS failure, TLS error) or returned a non-JSON body, the per-recipient result was just "Unknown error" — with no way to tell why delivery failed.

This PR:

  1. Adds errorCode (cURL errno) to the result shape returned by Adapter::request() and Adapter::requestMulti().
  2. Extracts FCM error resolution into a getError() helper that falls back to transport-level details when Firebase doesn't provide an error message.

Why

A failed FCM send with statusCode: 0 and response: null (e.g. connection timeout) produced:

{ "recipient": "token123", "error": "Unknown error" }

With this change, the same failure produces:

{ "recipient": "token123", "error": "Connection timed out after 10000 milliseconds (HTTP status 0; cURL error code 28)" }

This makes production delivery failures actually diagnosable from message logs.

How

Adapter.php — capture cURL error codes

Single requests use curl_errno(), multi requests use the per-handle result from curl_multi_info_read():

return [
    'url' => $url,
    'statusCode' => ...,
    'response' => $response,
    'headers' => $responseHeaders,
    'error' => \curl_error($ch),
    'errorCode' => \curl_errno($ch),   // new
];

The requestMulti() docblock is also corrected: response can be string|null as well as array (it already could at runtime — the annotation was wrong).

FCM.php — layered error resolution

The inline ternary chain in process() is replaced with a getError() helper that resolves errors in priority order:

protected function getError(array $result): string
{
    $response = \is_array($result['response']) ? $result['response'] : [];
    $error = \is_array($response['error'] ?? null) ? $response['error'] : [];

    // 1. Expired/unregistered token → canonical expired message
    if (\in_array($error['status'] ?? null, ['UNREGISTERED', 'NOT_FOUND'], true)) {
        return $this->getExpiredErrorMessage();
    }

    // 2. Firebase-provided error message
    $message = $error['message'] ?? null;
    if (\is_string($message) && $message !== '') {
        return $message;
    }

    // 3. Transport error + diagnostic codes
    $transportError = $result['error'] ?? null;
    $details = "HTTP status {$result['statusCode']}; cURL error code {$result['errorCode']}";

    return \is_string($transportError) && $transportError !== ''
        ? "{$transportError} ({$details})"
        : "Request failed ({$details})";
}

This also hardens against non-array responses (e.g. an HTML error page from a proxy), which previously relied on null coalescing over a string.

Tests

FCMTest gains a data-provider unit test covering all four resolution paths (runs without FCM credentials, unlike the existing integration tests):

Case Input Expected error
Firebase error message response.error.message set Invalid registration token
Expired token error.status = NOT_FOUND Expired device token
Transport failure cURL timeout, errno 28 Connection timed out after 10000 milliseconds (HTTP status 0; cURL error code 28)
No message at all HTTP 503, string body Request failed (HTTP status 503; cURL error code 0)

Existing test stubs (ResendRoutingTest, SESRoutingTest, Msg91Test) are updated to include errorCode in their stubbed result shapes.

$ composer test   → OK
$ composer lint   → pass
$ composer analyse → [OK] No errors (phpstan level 6)

Breaking changes

None for consumers of adapters. Subclasses that override request()/requestMulti() and rely on the documented return shape should add errorCode (see updated test stubs for reference).

@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR improves FCM push delivery error reporting by surfacing transport-layer failures (timeouts, DNS errors, TLS issues) that previously produced an opaque "Unknown error" result. It also hardens the error-extraction logic against non-array responses such as HTML error pages from proxies.

  • Adapter.php: Adds errorCode: int to both request() and requestMulti() return shapes — single requests use curl_errno($ch), multi requests use $info['result'] from curl_multi_info_read() (the CURLcode for that completed handle, semantically equivalent).
  • FCM.php: Extracts inline error resolution into a getError() helper with a clear priority order — expired-token detection → Firebase message → transport details — and correctly guards against non-array response bodies before accessing nested keys.
  • Tests: A new data-provider test covers all four resolution paths without requiring live FCM credentials; existing test stubs are updated to include errorCode: 0 to keep their return shapes in sync.

Confidence Score: 5/5

Safe to merge — changes are additive, backward-compatible, and confined to error-reporting paths that have no effect on successful deliveries.

The transport-error capture is straightforward: curl_errno() for single requests and $info['result'] (the CURLcode) for multi requests are semantically identical. The getError() helper correctly guards against null/string responses before accessing nested array keys, fixing a latent warning risk in the old code. Tests cover all four resolution paths and all affected stubs are updated. No behavioural change occurs on the success path.

No files require special attention.

Important Files Changed

Filename Overview
src/Utopia/Messaging/Adapter.php Adds errorCode to both request() and requestMulti() return shapes; single-request path uses curl_errno($ch), multi path correctly uses $info['result'] from curl_multi_info_read().
src/Utopia/Messaging/Adapter/Push/FCM.php Replaces inline ternary error resolution with a getError() helper; adds UNREGISTERED/NOT_FOUND token detection, Firebase message extraction, and transport-level fallback with cURL errno/status detail.
tests/Messaging/Adapter/Push/FCMTest.php Adds a data-provider unit test for getError() covering all four resolution paths via an anonymous subclass; no credentials required.
tests/Messaging/Adapter/Email/ResendRoutingTest.php Stub updated to include errorCode: 0 in its return shape and updated docblock annotation.
tests/Messaging/Adapter/Email/SESRoutingTest.php Stub updated to include errorCode: 0 in its return shape and updated docblock annotation.
tests/Messaging/Adapter/SMS/Msg91Test.php Stub updated to include errorCode: 0 in its return shape and updated docblock annotation.

Reviews (1): Last reviewed commit: "(feat): surface transport error details ..." | Re-trigger Greptile

@ChiragAgg5k ChiragAgg5k merged commit 63a27be into main Jul 14, 2026
3 of 4 checks passed
@ChiragAgg5k ChiragAgg5k deleted the feat/fcm-error-details branch July 14, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants