Skip to content

fix http server timeouts - #596

Draft
mirecta wants to merge 1 commit into
esp-rs:masterfrom
mirecta:master
Draft

fix http server timeouts#596
mirecta wants to merge 1 commit into
esp-rs:masterfrom
mirecta:master

Conversation

@mirecta

@mirecta mirecta commented Aug 12, 2025

Copy link
Copy Markdown

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • [* ] I have updated existing examples or added new ones (if applicable).
  • [ *] I have used cargo fmt command to ensure that all changed code is formatted correctly.
  • [ *] I have used cargo clippy command to ensure that all changed code passes latest Clippy nightly lints.
  • [ *] My changes were added to the CHANGELOG.md in the proper section.

Pull Request Details 📖

Description

Now HTTP server accept timeouts from config not hardcoded 5 seconds

Testing

I set timeout for large file which receive exceed 5 seconds and it works now

Comment thread src/http/server.rs
lru_purge_enable: conf.lru_purge_enable,
recv_wait_timeout: 5,
send_wait_timeout: 5,
recv_wait_timeout: conf.session_timeout.as_secs() as u16,

@ivmarkov ivmarkov Aug 13, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think max_sessions and session_timeout (which are currently unused) are remnants from the old HTTP server, which did aim at supporting HTTP sessions.

Perhaps it is not a very wise idea to reuse something called session_timeout for TCP socket receive/send timeout.

I would rather suggest that you introduce two additional Option<Duration> parameters, named send_wait_timeout and recv_wait_timeout, and you use those instead. And delete the session_timeout and max_sessions unused parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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