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

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

Open
Sinc63 opened this issue Jun 25, 2024 · 0 comments

Comments

@Sinc63
Copy link

Sinc63 commented Jun 25, 2024

Environment

  • Elixir & Erlang/OTP versions (elixir --version):
    Erlang/OTP 26 [erts-14.2.5] [source] [64-bit] [smp:2:2] [ds:2:2:10] [async-threads:1] [jit:ns]

Elixir 1.16.3 (compiled with Erlang/OTP 26)

  • Operating system:
    Oracle Linux 9
  • Pigeon version:
    2.0.0-rc.2

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.

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

No branches or pull requests

1 participant