search

J0 / phoenix_gen_socket_client

Socket client behaviour for phoenix channels
MIT License
232 stars 48 forks source link

Stop crashing on encoding errors #21

Closed obrok closed 7 years ago

obrok commented 7 years ago

To complete https://github.com/Aircloak/aircloak/issues/1700 it would be much easier if you could get the error as a tuple instead of the having to rescue - @sasa1977 WDYT?

obrok commented 7 years ago

@sasa1977 is on vacation, so @cristianberneanu - please review

obrok commented 7 years ago

I am not a fan of the change since it modifies the API

The API already allowed {:error, _} tuples, so at least it doesn't change that...

Rescue-ing the exception isn't that bad.

The original exception is a bit of an unknown quantity. Maybe it will be a {:invalid, _} from Poison or maybe a :bad_argument - it will be difficult to rescue only encoding errors this way.

obrok commented 7 years ago

@cristianberneanu if you're unsure about this direction, maybe you'd prefer to wait until @sasa1977 comes back to express an opinion?

cristianberneanu commented 7 years ago

Since I already approved, you can merge at will. I am happier now that the original exception is preserved.

obrok commented 7 years ago

@cristianberneanu I bumped the version in preparation for release - please re-approve

obrok commented 7 years ago

Well, I guess you don't need to approve in this repo, but let me know if you have any further comments.

cristianberneanu commented 7 years ago

I don't have anything to add, you can go ahead and merge.