codedge-llc / pigeon

iOS and Android push notifications for Elixir
https://hex.pm/packages/pigeon
MIT License
639 stars 143 forks source link

responses that return a Notification should remove the __meta__ field from the Notification #260

Open Sinc63 opened 3 months ago

Sinc63 commented 3 months ago

Environment

Elixir 1.16.3 (compiled with Erlang/OTP 26)

Current behavior

I upgraded my push code to Pigeon 2.0 to get the benefit of the new FCM model, and resolve several language changes for Elixir 1.16. I just spent some time trying to figure out why my test cases were giving me Jason problems. I have now reached a solid conclusion. In my Phoenix controller to pass push notifications through from several applications to Apple and Google I have a response handler to deal with the success and error cases. Each of these returns the full response that I receive from Pigeon, with a few modifications as needed. Because Pigeon 2.0 now inserts the on_response function into metadata of the Notification, that metadata is returned in my responses, and can't be encoded by Jason.

I believe that the correct solution to this is that the Tasks module should delete the metadata field from the notification in the three versions of the function that return a notification. The field was added as an internal convenience for Pigeon and shouldn't be returned to users once it is no longer required.

I know, PRs are welcome. I just might, especially if you will soon release 2.0 with an upgrade guide, which would be tremendously helpful to my current efforts.

Include code samples, errors and stacktraces if appropriate.

     ** (Protocol.UndefinedError) protocol Jason.Encoder not implemented for %Pigeon.Metadata{on_response: #Function<0.98353279/1 in PushServerWeb.API.ApnsController.handle_response_fn/2>} of type Pigeon.Metadata (a struct), Jason.Encoder protocol must always be explicitly implemented.

     If you own the struct, you can derive the implementation specifying which fields should be encoded to JSON:

         @derive {Jason.Encoder, only: [....]}
         defstruct ...

     It is also possible to encode all fields, although this should be used carefully to avoid accidentally leaking private information when new fields are added:

         @derive Jason.Encoder
         defstruct ...

     Finally, if you don't own the struct you want to encode to JSON, you may use Protocol.derive/3 placed outside of any module:

         Protocol.derive(Jason.Encoder, NameOfTheStruct, only: [...])
         Protocol.derive(Jason.Encoder, NameOfTheStruct)
     . This protocol is implemented for the following type(s): Any, Atom, BitString, Date, DateTime, Decimal, Ecto.Association.NotLoaded, Ecto.Schema.Metadata, Float, Integer, Jason.Fragment, Jason.OrderedObject, List, Map, NaiveDateTime, Time
     code: |> post("/api/v1/apns/push", params)
     stacktrace:
       (jason 1.4.1) lib/jason.ex:213: Jason.encode_to_iodata!/2
       (phoenix 1.7.12) lib/phoenix/controller.ex:366: Phoenix.Controller.json/2

Expected behavior

No problems.