Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Add ExecutionTimeout parameter #484

Closed
benaadams opened this issue Dec 14, 2015 · 9 comments
Closed

Add ExecutionTimeout parameter #484

benaadams opened this issue Dec 14, 2015 · 9 comments
Labels
Milestone

Comments

@benaadams
Copy link
Contributor

Timeout between two successive body reads which if exceeded the client is determined to be disconnected and the request aborted.

Failure state: 408 Request Timeout

ngnix default client_body_timeout 60s

Implementation-wise the timer can use the same timer as Keep-alive; as a multi stage/state timer.

When deployed in recommended configuration with a reverse proxy (e.g. IIS/nginx) this would be handled by the proxy - however, this wouldn't apply in a pure Kestrel deployment.

@Tratcher
Copy link
Member

Make sure this doesn't break websockets.

@benaadams
Copy link
Contributor Author

Re: websockets @Tratcher I don't think so; but https isn't an upgrade request is it?

@Tratcher
Copy link
Member

https isn't an upgrade request is it?

Huh?

@benaadams
Copy link
Contributor Author

Huh?

My plan is to switch off the timers on opaque stream upgrade and assume they were handling that interaction (would also need to be same for MaxUploadBytes) - just making sure there is no corner cases I hadn't thought of (https in this case)

@Tratcher
Copy link
Member

Ah, sounds good.

https shouldn't be any different from http in this scenario.

@benaadams
Copy link
Contributor Author

Would be easier to have this as a maxRequestExecutionTimeout, that includes body read time, that work?

Would probably also be more useful that way.

/cc @blowdart @halter73 @CesarBS @muratg

@benaadams benaadams changed the title Add BodyTimeout parameter Add ExecutionTimeout parameter Jan 12, 2016
@benaadams
Copy link
Contributor Author

Hmmm... may cause issue with large file uploads; so may still need a sliding window for body requests :-/

@blowdart
Copy link
Member

Hurrah, timeouts :D

@DamianEdwards
Copy link
Member

post RC-2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants