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

Proper TCP socket shutdown; Generic TCP timeout utils; built-in HTTP server timeouts; update docu #34

Merged
merged 10 commits into from
Oct 8, 2024

Conversation

ivmarkov
Copy link
Owner

@ivmarkov ivmarkov commented Oct 8, 2024

This PR implements:

  • A TcpShutdown trait with proper socket close and abort functionality for both STD and embassy-net. Needs testing for embassy-net
  • Generic TCP "timeout" decorators in edge_net which can be used by anyone
  • Built-in timeouts (both for each IO operation, as well as for the whole request-response) for the HTTP server

@ivmarkov ivmarkov changed the title Proper TCP socket shutdown; update docu Proper TCP socket shutdown; Generic TCP timeout utils; built-in HTTP server timeouts; update docu Oct 8, 2024
@AnthonyGrondin
Copy link
Contributor

I tested with esp-wifi + embassy-net and I can confirm that this fixes the issues I've been having with closing connections:

Here's a trace for a request issued with "Connection: Close" and following the redirection.

image

Here's a trace for a request issued with "Connection: Keep-Alive" and following the redirection.
image

@AnthonyGrondin
Copy link
Contributor

LGTM.
Thank you very much for taking care of this.

If some additional optional parameters are ever added to run(), it may become more user friendly to switch to a builder pattern, but at this point, this is more about UX than usability, and we should focus on usability for now.

@ivmarkov
Copy link
Owner Author

ivmarkov commented Oct 8, 2024

If some additional optional parameters are ever added to run(), it may become more user friendly to switch to a builder pattern, but at this point, this is more about UX than usability, and we should focus on usability for now.

Agreed. A builder pattern can wait a bit.

@ivmarkov ivmarkov merged commit 6e017d9 into master Oct 8, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants