search

eclipse-uprotocol / up-streamer-rust

Generic uStreamer implementation written in Rust
Apache License 2.0
1 stars 13 forks source link

Error handling when register_listener fails #21

Closed jjj-vtm closed 3 months ago

jjj-vtm commented 4 months ago

If register_listener fails there is only a log message

https://github.com/eclipse-uprotocol/up-streamer-rust/blob/d4ab17fbc745705290e29e5251fa8e9fcbffe943/up-streamer/src/ustreamer.rs#L153C1-L173

but the constructed tuple is added to forwarding_listeners and set to active. This basically leaves a "dead" listener in the HashMap.

PLeVasseur commented 4 months ago

Thanks for pointing this out! You're completely right :)

pranavishere2 commented 3 months ago

@PLeVasseur Please assign this to me

PLeVasseur commented 3 months ago

@pranavishere2 -- you mentioned yesterday some refactoring done in this area fixed this issue.

Could you update here with info on how it was solved?

Perhaps you could also write a test to prove this out.

pranavishere2 commented 3 months ago

Prior to the update in the main branch, the following code was leading to a dead listener object even when registerListener failed:

https://github.com/eclipse-uprotocol/up-streamer-rust/blob/d4ab17fbc745705290e29e5251fa8e9fcbffe943/up-streamer/src/ustreamer.rs#L153C1-L173

Following the refactoring in the main branch, this issue has been resolved by introducing an exception when the registerListener call fails. The updated implementation ensures that if the listener cannot be registered, an error is thrown to prevent the dead listener object. Below is the code snippet:

https://github.com/eclipse-uprotocol/up-streamer-rust/blob/541160edb5c4567cdcfa6c489dd7656c905fa678/up-streamer/src/ustreamer.rs#L216C8-L244C100

Also the listener is added in the map at the end of the function only when all preconditions are satisfied: https://github.com/eclipse-uprotocol/up-streamer-rust/blob/541160edb5c4567cdcfa6c489dd7656c905fa678/up-streamer/src/ustreamer.rs#L302C1-L307C38

This makes sure there will not be any dead listner when registerListener API fails.

PLeVasseur commented 3 months ago

@pranavishere2 -- please use code tags to put the code in and give references to the main branch which files / lines, i.e. use links.

pranavishere2 commented 3 months ago

@pranavishere2 -- please use code tags to put the code in and give references to the main branch which files / lines, i.e. use links.

Done

PLeVasseur commented 3 months ago

Thanks @pranavishere2 -- closing as completed by #20