Skip to content
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

Allow http client cancel request (IDFGH-8426) #9892

Closed
hoangdovan opened this issue Oct 1, 2022 · 11 comments
Closed

Allow http client cancel request (IDFGH-8426) #9892

hoangdovan opened this issue Oct 1, 2022 · 11 comments
Assignees
Labels
Awaiting Response awaiting a response from the author Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@hoangdovan
Copy link

hoangdovan commented Oct 1, 2022

Is your feature request related to a problem?

Sometime we need cancel a http request which already sent out (receiving response not completed yet). But the current esp-idf http client API does not include any function to allow cancel http request. So if the response from server is so long, program will be stuck there, waiting until response completed.

Describe the solution you'd like.

Http client API should add function to allow cancel http request which already sent out, discard response so CPU can be free and do another task immediately.

Describe alternatives you've considered.

No response

Additional context.

No response

@hoangdovan hoangdovan added the Type: Feature Request Feature request for IDF label Oct 1, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label Oct 1, 2022
@github-actions github-actions bot changed the title Allow http client cancel request Allow http client cancel request (IDFGH-8426) Oct 1, 2022
@kayzen9
Copy link

kayzen9 commented Oct 9, 2022

I use esp_http_client_flush_response API for situations where response from server should be discarded. It is not possible to cancel the request, unless connection itself is aborted (using esp_http_client_close). Or am I missing something here?

@espressif-bot espressif-bot assigned hmalpani and unassigned mahavirj Oct 10, 2022
@espressif-bot espressif-bot added the Awaiting Response awaiting a response from the author label Oct 18, 2022
@hoangdovan
Copy link
Author

@kayzen9 I don't think you can cancel http request properly unless Espressif support this API in esp-idf.

@hmalpani
Copy link
Contributor

hmalpani commented Nov 2, 2022

Hello @hoangdovan ,
Canceling HTTP requests is not supported by http_client in IDF. In your case, you can close the connection and open it again. You can also use esp_http_client_flush_response as mentioned by @kayzen9 to discard the response.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Nov 2, 2022
@KaeLL
Copy link
Contributor

KaeLL commented Nov 2, 2022

@hmalpani OP seems to know it's not supported, hence the Feature Request label.

@hoangdovan
Copy link
Author

Hello @hoangdovan , Canceling HTTP requests is not supported by http_client in IDF. In your case, you can close the connection and open it again. You can also use esp_http_client_flush_response as mentioned by @kayzen9 to discard the response.

@hmalpani You mean esp-idf will not support this feature in the future?

@hmalpani
Copy link
Contributor

hmalpani commented Nov 7, 2022

Hello @hoangdovan

esp_http_client supports requests in both blocking and non-blocking manner. To add support for canceling the HTTP request, we will need to spawn a background task that will consume the server's response. It should be a nonblocking API for this purpose. It should then send some event to inform the client that the request has been canceled. This implementation seems like it will complicate the component. I had a discussion internally and we recommend canceling the HTTP request on the user application side as more feasible.

Also in case you have some method to cancel the HTTP request, please feel free to raise a PR. I will be happy to take a look at it.

@hoangdovan
Copy link
Author

hoangdovan commented Nov 7, 2022

Hello @hmalpani now I'm digging into esp-idf source code too, to find if there is any idea for cancel http request. But if something like you claim: "To add support for canceling the HTTP request, we will need to spawn a background task that will consume the server's response", I think the cost to do this is too big. It will increase memory using, CPU still need to working to handle response from server which we want to cancel.
My question is why we still need to consume server response?
I know that http request is stateless and cannot cancel easily.
But at lower level TCP/IP, it seem it's possible to close connection and then stop server from continue sending response to client?
Or something I am wrong here?

@hmalpani
Copy link
Contributor

Hello @hoangdovan
Sharing a patch here to add the functionality for canceling HTTP requests. Can you verify if this works with your code?
IDFGH-8426.txt

@hoangdovan
Copy link
Author

Hello @hoangdovan Sharing a patch here to add the functionality for canceling HTTP requests. Can you verify if this works with your code? IDFGH-8426.txt

Hello @hmalpani
After some testing, I see it work with my code!
In my app, I do testing cancel http request while download big file from server.
Using esp_http_client_cancel_request() in http event handler (HTTP_EVENT_ON_DATA).
It work as expected.
So will you officially add this API to esp-idf? And I will close this issue here?
Thank you!

@hmalpani
Copy link
Contributor

Hello @hoangdovan
Thanks for testing the feature. Yes, we have an internal MR to add this functionality. It will be available soon on the master branch of GitHub. This issue will be closed automatically once the feature is available on GitHub.

@hoangdovan
Copy link
Author

@hmalpani
Thank you very much for your information!

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Response awaiting a response from the author Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

6 participants