Skip to content

Conversation

mirecta
Copy link

@mirecta mirecta commented Aug 12, 2025

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

@@ -153,8 +153,8 @@ impl From<&Configuration> for Newtype<httpd_config_t> {
max_resp_headers: conf.max_resp_headers as _,
backlog_conn: 5,
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,
Copy link
Collaborator

@ivmarkov ivmarkov Aug 13, 2025

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
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