unleash-client-rust
unleash-client-rust copied to clipboard
Feat: Preserve original error message in warn! logs
This PR rearranges the error handling in poll_for_updates
. It preserves the original error to aid in debugging.
About the changes
Closes #35 .
Important files
Only one file was touched.
Discussion points
The original code collected whether an error happened in a var that is later checked and logged. I instead embedded the warn!
calls right where errors happen. I believe this aids in clarity and also means there can be slightly different error messages depending on the situation, but I am open to other ways of handling it as well.
I've tried a different approach, lifting all the update logic into its own fn and return
ing whenever an error is encountered. It's certainly much less nested, but I'm not sure if it addresses the "key ergonomics around error handling" issue. I would appreciate some feedback!
With regards to opentelemetry, I'm hesitant to implement what sounds like will be a fairly large change. My intention was to do a small, in-place (which i suppose is no longer applicable), uncontroversial fix that would immediately improve my use-case. While I think it would be ideal to integrate observability, unleash-client-rust
is already logging here. I don't feel like it would hurt to make those existing logs slightly better.
I have pushed a change that makes non-JSON responses from the features endpoint be more obvious, a la #37
Thank you. The tokio patch is merged now, and I'll try to find time to review this in detail later this week or next.