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

Add timeouts to response handling #17

Merged
merged 3 commits into from
Nov 18, 2020
Merged

Conversation

Alch-Emi
Copy link
Contributor

@Alch-Emi Alch-Emi commented Nov 18, 2020

This is a very preliminary attempt at handling #9.

From the docs:

Set the timeout on incoming requests

Note that this timeout is applied twice, once for the delivery of the request, and once for sending the client's response. This means that for a 1 second timeout, the client will have 1 second to complete the TLS handshake and deliver a request header, then your API will have as much time as it needs to handle the request, before the client has another second to receive the response.

If you would like a timeout for your code itself, please use tokio::time::Timeout to implement it internally.

The default timeout is 1 second.

It's worth noting that this leaves the questions from the original issue pretty unaddressed, specifically with how they should be worked into the API. In this PR, timeouts are set using the Builder for the Server, with no callbacks that might allow a client to handle them. I'm happy to write such a handler, but I'm not sure how best to go about it, or even what the usecases might be. What sort of things do you think a user would want access to when handling a timeout?

Also, I'm not really a security expert, so if this is not a safe enough timeout handling strategy, please let me know


This change is Reviewable

@panicbit
Copy link
Owner

Thanks! I'll review this one in ~10 hours ⌚ 👀

@panicbit
Copy link
Owner

image

@panicbit panicbit merged commit 4eae63a into panicbit:master Nov 18, 2020
@panicbit
Copy link
Owner

panicbit commented Nov 18, 2020

Turns out there's a reason for the existence of the HEAD method in http. Timeouts in Gemini are kinda tricky:
image
In this picture the request times out because the client is asking the user how to open the file.

@panicbit
Copy link
Owner

I guess this could be fixed by a different client behaviour, but all of this is very underspecified.

@panicbit panicbit mentioned this pull request Nov 18, 2020
@Alch-Emi Alch-Emi deleted the timeout branch November 19, 2020 07:03
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