Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(FCM.Error): handle multiple elements in "details" list with recursive parsing #293

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

caruspi
Copy link

@caruspi caruspi commented Feb 25, 2025

Description

The current implementation of the parse/1 function in Pigeon.FCM.Error does not correctly handle cases where the "details" list contains multiple elements. It only matches when only one element is in the list and has an "errorCode", causing certain errors to be mapped as :unknown_error instead of the expected values.

In our production logs, we are seeing a large number of :unknown_error entries with the following error message:

%{
  "code" => 404,
  "details" => [
    %{
      "@type" => "type.googleapis.com/google.firebase.fcm.v1.FcmError",
      "errorCode" => "UNREGISTERED"
    },
    %{
      "@type" => "type.googleapis.com/google.firebase.fcm.v1.ApnsError",
      "reason" => "Unregistered",
      "statusCode" => 410
    }
  ],
  "message" => "Requested entity was not found.",
  "status" => "NOT_FOUND"
}

This should be mapped as :unregistered, but the current implementation fails to find the "errorCode" because multiple elements are in the list.

Solution

This PR refactors the parse/1 function to use a recursive approach for parsing the "details" list. It now iterates through all elements until it finds one with an "errorCode" and correctly maps it to the corresponding atom. If no "errorCode" is found, it falls back to checking the "status" field.

…ments in "details"

Refactored the `parse/1` function to use recursion for correctly handling cases where the "details" list contains multiple elements, and falling back to the "status" field when necessary.
@caruspi caruspi force-pushed the fix/fcm-error-handling branch from 7aca252 to c19f5b0 Compare February 25, 2025 16:53
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.

1 participant