crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.42k stars 1.62k forks source link

[RFC] Allow setting timeouts on `HTTP::Server::Response` #14799

Open Blacksmoke16 opened 3 months ago

Blacksmoke16 commented 3 months ago

Creating this on behalf of @m-o-e related to a discussion on gitter.

A HTTP::Server::Response is backed by default via a Socket when using HTTP::Server. The proposed use case is making it easier to define an explicit timeout on the underlying response by providing a #write_timeout= method on the server response obj itself. The way they went about this was like ctx.response.output.as(HTTP::Server::Response::Output).@io.as(Socket).write_timeout = 1.second.

One potential problem with this is that there is no requirement that an http response is backed by a Socket. The setter could ensure the #output responds to :write_timeout=, and no-ops if it does not. Ideally there'd maybe be an interface for timeoutable IOs, but thats prob for another discussion. I also suggested it would be easier to do this at a reverse proxy level like nginx, but do agree that you shouldn't have to need to set one of those up just to do this.

Either way, wanted to at least get something created for posterity for future discussion.

m-o-e commented 3 months ago
require "http/server"

# 1. Run this server
#
# 2. In second terminal run:
#    curl -Nv http://localhost:9876
#
# 3. Hit ^Z to suspend the curl
#
# 4. Observe in first terminal what happens to the server.
#    WITHOUT timeout: Server blocks indefinitely
#    WITH timeout: <HTTP::Server::ClientError:Error while flushing data to the client>

server = HTTP::Server.new do |ctx|
  # Uncomment the following line to test the "WITH timeout" case
  # ctx.response.output.as(HTTP::Server::Response::Output).@io.as(Socket).write_timeout = 1.second

  loop do
    ctx.response.print("hello")
    puts Time.utc
  end
rescue ex : Exception
  p! ex
end

# Start the server
address = "127.0.0.1"
port = 9876
puts "Listening on http://#{address}:#{port}"
server.bind_tcp(address, port)
server.listen
dsisnero commented 3 months ago

I think we should abstract timeout and such further to cancellation tokens. Please read this article https://vorpus.org/blog/timeouts-and-cancellation-for-humans/ which explains the difference in detail.