Skip to content

Commit

Permalink
Switch to using a new HTTP client for each twirp client request
Browse files Browse the repository at this point in the history
Ideally Crystal's HTTP client would manage a pool of connections and reuse them when appropriate, but it does not – see crystal-lang/crystal#6011 – so using a new TCP connection for each request seems like the safest approach.

The previous implementation was hopelessly broken due to the `@client.close`, which resulted in `Closed stream (IO::Error)` errors when used concurrently... However, IIRC, without the explicit close other issues arise when TCP sockets timeout of their own accord.
  • Loading branch information
mloughran committed Jul 27, 2022
1 parent 5c368f2 commit ce72180
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions src/twirp/client.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,26 @@ require "./error"

module Twirp
class Client(T)
@callback : (HTTP::Request ->)?

def initialize(@uri : URI)
@client = HTTP::Client.new(uri)
@prefix = uri.path.presence || "/twirp"
end

# Adds a callback to execute before each request (see `HTTP::Client#before_request`)
def before_request(&callback : HTTP::Request ->) : Nil
@client.before_request(&callback)
@callback = callback
end

def call(rpc_name, request, response_type)
begin
response = @client.post("#{@prefix}/#{T.service_name}/#{rpc_name}",
response = HTTP::Client.new(@uri) do |client|
if cb = @callback
client.before_request(&cb)
end
client.post("#{@prefix}/#{T.service_name}/#{rpc_name}",
headers: HTTP::Headers{"Content-Type" => "application/protobuf"},
body: request.to_protobuf,
)
ensure
@client.close
end

unless body = response.body?
Expand Down

0 comments on commit ce72180

Please sign in to comment.