fix (python): handle cancelled requests gracefully #5080
Conversation
|
Hi @invokermain! Thanks for contributing. We will try and get to it asap. As a side note though, It looks like the DCO for our CICD has failed. That is because we require all commits to have both DCO sign-off, as well as signing. |
invokermain
force-pushed
the
python-async-invalid-state
branch
from
6824aca to
26239f4
Compare
|
no problem I have amended & signed the commit |
@invokermain It looks like you have DCO approval, but your previous commit is not signed/verified with a signature |
|
@invokermain Thanks for the fix, it generally looks good! However, please resolve the Python CICD issues |
invokermain
force-pushed
the
python-async-invalid-state
branch
from
d2b69c0 to
ae05f1b
Compare
|
I think my local keys are incorrect (wrong email), I will take a look again later today |
invokermain
force-pushed
the
python-async-invalid-state
branch
from
ae05f1b to
1a2b9da
Compare
Signed-off-by: invokermain <timothyj725@googlemail.com>
invokermain
force-pushed
the
python-async-invalid-state
branch
from
1a2b9da to
dda9d46
Compare
|
cool, I just need (re) approval on the workflow so I can see what the issues are. Then I can fix them. |
Signed-off-by: invokermain <timothyj725@googlemail.com>
invokermain
force-pushed
the
python-async-invalid-state
branch
from
9435a52 to
cbdc448
Compare
Signed-off-by: invokermain <timothyj725@googlemail.com>
|
I've simplified the fix to be a more minimal change, that fixes the failing test cases locally. |
* fix (python): handle cancelled requests gracefully Signed-off-by: invokermain <timothyj725@googlemail.com>
4 participants
Issue link
This Pull Request is linked to issue (URL): #4653
Description
The Python async_client does not handle cancellation correctly. This issue would affect both async backends (asyncio/trio) in different ways.
When a request is cancelled by the client, the server will still send a response which will trigger the
_process_responselogic. However, when using asyncio, the future will be marked as done due to the cancellation. Note, trio behaves differently here due to_CompatFuture, which will not be in the done state when cancelled.The current code would not check whether the future is done, and will try to call
set_resulton the future. In asyncio this raises anInvalidStateErrorwhich would then cause the client to be closed.Fix
The fix is relatively simple, we explicitly check whether the future is done, and if so ignore it (as we know no one is going to read the response anyway).
The current code would also close the client if an orphaned/unknown response was received, this seems needlessly defensive, so this PR changes that behaviour to just log and ignore it.