You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hi @kyrylo
I'm using Airbrake-Rust in an environment with really bad network connection, so I'm in a desperate need of a new feature to reliably send error notifications. I'm thinking about adding a "retry" feature with some configurable timeouts and number of maximum retries.
Before implementing something this big I would like to get your feedback on the idea and discuss the implementation details.
A: We will definitely need better error handling in the lib to determine if a notice was sent successfully or not. I've tried to add a simple boilerplate Error type with the error_chain! macro, and replace every unwrap() in SyncSender::send() with a try!(). It works fine, except for the serde_json::from_str call, which has some extra requirements which I couldn't handle, but I will look into it a bit more.
Do you mind changing the notify_sync function to return a Result instead a Value? I hope it's not a problem, after all if a developer wants to block the app while the notification is beeing sent, he would probably want to know if it was successfull or not (and also don't want his program to die in case of an unexpected response of course)
B: Where should I put the retry logic? I think AsyncSender could have the same retry functionality, because it basically includes a SyncSender in a separate thread, so that thread can also do the retry thing, even if we discard the final results after a given number of failures. So maybe we can have a simple wrapper type for SyncSender which contains the retry logic and the related params. Then the AsyncSender worker should decide if a simple SyncSender or a new Retry Sender should be created based on the config params. In this case the Notifier would have 3 senders, Sync, RetrySync and Async.
The other option is to not separate this, and make the SyncSender smarter by adding the retry logic to the send() function. I mean it gets the whole config in the constructor, so it can easily decide itself if any retries are needed. This way the whole structure can be the same, and we only have to add some new optional config params.
Both methods are backward compatible with the API.
The text was updated successfully, but these errors were encountered:
I've managed to get the error_chain feature working, so you can check out a working prototype in my repo on branch: retry_prototype
I went with the second option, making the send() function in SyncSender smarter to handle retries, so AsyncSender doesn't have to know anything about the new retry functionality, it just passes the config.
If it's looking good on your side I will make this change into separate, fully fledged pull requests with updating the Readme and everything, just wanted to get your opinion first.
Hi @kyrylo
I'm using Airbrake-Rust in an environment with really bad network connection, so I'm in a desperate need of a new feature to reliably send error notifications. I'm thinking about adding a "retry" feature with some configurable timeouts and number of maximum retries.
Before implementing something this big I would like to get your feedback on the idea and discuss the implementation details.
A: We will definitely need better error handling in the lib to determine if a notice was sent successfully or not. I've tried to add a simple boilerplate Error type with the error_chain! macro, and replace every unwrap() in SyncSender::send() with a try!(). It works fine, except for the serde_json::from_str call, which has some extra requirements which I couldn't handle, but I will look into it a bit more.
Do you mind changing the notify_sync function to return a Result instead a Value? I hope it's not a problem, after all if a developer wants to block the app while the notification is beeing sent, he would probably want to know if it was successfull or not (and also don't want his program to die in case of an unexpected response of course)
B: Where should I put the retry logic? I think AsyncSender could have the same retry functionality, because it basically includes a SyncSender in a separate thread, so that thread can also do the retry thing, even if we discard the final results after a given number of failures. So maybe we can have a simple wrapper type for SyncSender which contains the retry logic and the related params. Then the AsyncSender worker should decide if a simple SyncSender or a new Retry Sender should be created based on the config params. In this case the Notifier would have 3 senders, Sync, RetrySync and Async.
The other option is to not separate this, and make the SyncSender smarter by adding the retry logic to the send() function. I mean it gets the whole config in the constructor, so it can easily decide itself if any retries are needed. This way the whole structure can be the same, and we only have to add some new optional config params.
Both methods are backward compatible with the API.
The text was updated successfully, but these errors were encountered: