Payload decode error fix and enhancement

smartrent / jackalope

An opinionated MQTT client library based on Tortoise MQTT

Apache License 2.0

28 stars 5 forks source link

Payload decode error fix and enhancement #30

Closed jjcarstens closed 3 years ago

jjcarstens commented 3 years ago

Fix topic return type for :payload_decode_error
- The topic returned in payload_decode_error is the same "topic" in Tortoise.Handler.handle_message/3 which means it is a list of topic levels, but the type for Jackalope.Handler.handle_message/2 topic is a string.
Include Jason.DecodeError in :payload_decode_error
- I think it would be useful to include the Jason.DecodeError in the :payload_decode_error message so that it can be reviewed/reported by the other end if desired.

fhunleth commented 3 years ago

Can the other inconsistencies be fixed too?

For example, the variable is called topic at https://github.com/smartrent/jackalope/blob/main/lib/jackalope/tortoise_handler.ex#L47 where it should be called topic_levels like at https://github.com/gausby/tortoise/blob/master/lib/tortoise/handler.ex#L279.

I'm finding the code hard to review with the inconsistent naming and it looks like there's another topic/topic_levels mistake, but I'm not sure.

jjcarstens commented 3 years ago

Yes. This was a quick fix that i noticed while doing something else. I'll take a few more minutes to dive deeper into other places that are inconsistent as well

fhunleth commented 3 years ago

Thanks!

jjcarstens commented 3 years ago

@fhunleth I've gone through it and fixed the inconsistencies around topic vs topic_levels that I could see. Hopefully this is easier to review