jav / expo-server-sdk-java

Expo server SDK in java
MIT License
21 stars 23 forks source link

Additional Ticket Errors #31

Open celandro opened 3 years ago

celandro commented 3 years ago

I am having an issue with my outdated branch and was looking in master to see if it would be fixed but it seems like it would fail for a different reason

Currently TicketError only allows for DeviceNotRegistered. There exists at least one more case My current error: There was an unknown error with the FCM server. See this error's details for more information.: Details [error=ProviderError, sentAt=null, additionalProperties={fault=fcm}]

I do not think using enums is appropriate for error codes in this case as unknown errors will call json parse exceptions

jav commented 3 years ago

Hi!

I'll have a look.

I'm not ready to drop Enums just yet, but perhaps a datatype that allows for easy classification (ennums) and also allows for an arbitrary other error + error-string could be an option.

I'm not going to be able to dive in to it this week. However probably during next.

On Sun, 14 Feb 2021 at 21:57, Ryan Barker notifications@github.com wrote:

I am having an issue with my outdated branch and was looking in master to see if it would be fixed but it seems like it would fail for a different reason

Currently TicketError only allows for DeviceNotRegistered. There exists at least one more case My current error: There was an unknown error with the FCM server. See this error's details for more information.: Details [error=ProviderError, sentAt=null, additionalProperties={fault=fcm}]

I do not think using enums is appropriate for error codes in this case as unknown errors will call json parse exceptions

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jav/expo-server-sdk-java/issues/31, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACAQ2KP75EOWEEUREXOJUDS7A2JXANCNFSM4XTS63QQ .

celandro commented 3 years ago

Using an enum for a string field controlled by a third party is not going to work but there are some patterns that give you things similar to enums for known fields and unknown fields.

Simplest approach is to have a string field that is mapped to the json and an enum field that is not. In the setter for the string field, see if it's a known value and set the enum, otherwise set an unknown enum.

On Sun, Feb 14, 2021 at 11:28 PM Javier Ubillos notifications@github.com wrote:

Hi!

I'll have a look.

I'm not ready to drop Enums just yet, but perhaps a datatype that allows for easy classification (ennums) and also allows for an arbitrary other error + error-string could be an option.

I'm not going to be able to dive in to it this week. However probably during next.

On Sun, 14 Feb 2021 at 21:57, Ryan Barker notifications@github.com wrote:

I am having an issue with my outdated branch and was looking in master to see if it would be fixed but it seems like it would fail for a different reason

Currently TicketError only allows for DeviceNotRegistered. There exists at least one more case My current error: There was an unknown error with the FCM server. See this error's details for more information.: Details [error=ProviderError, sentAt=null, additionalProperties={fault=fcm}]

I do not think using enums is appropriate for error codes in this case as unknown errors will call json parse exceptions

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jav/expo-server-sdk-java/issues/31, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACAQ2KP75EOWEEUREXOJUDS7A2JXANCNFSM4XTS63QQ

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jav/expo-server-sdk-java/issues/31#issuecomment-779011668, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC7RACH4VHC6GFZYEX5VTLS7DEIFANCNFSM4XTS63QQ .

majugurci commented 3 years ago

Hi, is there any news on this? We are also facing the same issue.

jav commented 3 years ago

I would love to accept a PR on this topic. However, I won't have the time before summer to implement and test such a change myself.