huma icon indicating copy to clipboard operation
huma copied to clipboard

How to specify WriteTimeout per router endpoint?

Open j0urneyk opened this issue 1 year ago • 1 comments

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.

j0urneyk avatar Oct 21 '24 06:10 j0urneyk

@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 avatar Oct 23 '24 01:10 danielgtaylor

@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.)

j0urneyk avatar Oct 24 '24 08:10 j0urneyk