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

How to specify WriteTimeout per router endpoint? #620

Open
j0urneyK opened this issue Oct 21, 2024 · 2 comments
Open

How to specify WriteTimeout per router endpoint? #620

j0urneyK opened this issue Oct 21, 2024 · 2 comments
Labels
question Further information is requested

Comments

@j0urneyK
Copy link
Contributor

I want to specify a WriteTimeout for each router endpoint, but it seems like huma.Operation only allows setting the BodyReadTimeout.

Due to current business requirements, we have set a large WriteTimeout for the HTTP server itself, but there are some endpoints where the WriteTimeout needs to be shorter.

@danielgtaylor
Copy link
Owner

@j0urneyK there is an example of how you can do this using the huma.Context.BodyWriter() in the SSE implementation here: https://github.com/danielgtaylor/huma/blob/main/sse/sse.go#L162. You could probably do this using a resolver since the handler won't have access to the huma.Context.

This might be a nice thing to add as an option to huma.Operation but I'm not sure how to handle errors when a write timeout cannot be successfully set. Ideas?

@danielgtaylor danielgtaylor added the question Further information is requested label Oct 23, 2024
@j0urneyK
Copy link
Contributor Author

j0urneyK commented Oct 24, 2024

@danielgtaylor

Thank you so much for the detailed response.

Thanks to your explanation, I was able to confirm that the Write Deadline works properly in the Resolver or Middleware layer by referencing the SSE example.

I also agree that adding WriteTimeout as an option to huma.Operation would be extremely useful.

I would like to work on this if possible, but there are a couple of additional things I would like to discuss before doing so.

First, I understood your mention of a situation where an error is returned if the WriteTimeout setting fails.

This seems to be a similar consideration to why the huma.Register function didn’t handle errors when setting the Read Timeout using ctx.SetReadDeadline.

In this regard, would it be reasonable to consider logging a warning or error-level message when an error occurs, given that the code execution happens during the request processing?

If you think this is okay, i might want to consider treating ctx.SetReadDeadline the same way.

Additionally, the following suggestion pertains to a different aspect than WriteTimeout

As you know, WriteTimeout is a timeout that limits the time it takes to write data to the Response Body.

Thus, even if a WriteTimeout occurs, the connection for the request remains open until the API handler finishes execution and the response is returned.

I believe there may be cases where the connection should be terminated when a WriteTimeout occurs.

Therefore, I would like to suggest adding a feature that immediately closes the connection in the event of a WriteTimeout.

This would work in a manner similar to net/http's TimeoutHandler.

I also want to set a timeout of the same amount of time as WriteTimeout in the context of http.Request when this feature is enabled. I expect that this will allow the user to detect the timeout via the context.Context passed to the API handler and handle the timeout appropriately. What do you think?

This feature can be enabled as a separate option from WriteTimeout in huma.Operation, but i envision it to follow the time set in WriteTimeout if enabled. (That is, WriteTimeout, TimeoutHandler, and http.Request.Context will all have the same Timeout.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants