-
Notifications
You must be signed in to change notification settings - Fork 43
Adds support for asio::cancel_after #324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
This may still yield CI errors due to the race conditions fixed by #320, so this one should be merged before this. |
|
||
In this situation, we don't know if the request was processed by the server or not. | ||
And we have no way to know it. By default, the library mark these requests as | ||
failed with `asio::error::operation_aborted`. (TODO: do we want another error code here?). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the library mark these
Needs an s at the end.
do we want another error code here?
I think we do, otherwise the user might not know why it was canceled. Transforming operation_aborted
into e.g. unresponded_on_connection_lost
will make it clear.
req.get_config().cancel_if_unresponded = false; // retry | ||
|
||
// Makes sure that the key is set, even in the presence of network errors. | ||
// This may suspend for an unspecified period of time if the server is down. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may suspend for an unspecified period of time if the server is down.
I would rephrase this to something like
This will remain suspended until the server is capable of serving requests again e.g. after a process manager restarted the server.
|
||
private: | ||
// Function object to initiate the async ops that use asio::any_completion_handler. | ||
// Required for asio::cancel_after to work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this required? i.e. why wouldn't the lambda be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cancel_after (without a timer argument) requires an executor to construct a temporary timer for the timeout. This executor is taken from the initiation object. This gets done for free if you use async_compose, but needs to be done manually when using async_initiate.
cancel_if_unresponded
flagclose #226