From ce721802a5cee716d97a64d05563efc5d0962fc5 Mon Sep 17 00:00:00 2001
From: Martyn Loughran <martyn@mloughran.com>
Date: Wed, 27 Jul 2022 12:14:23 +0100
Subject: [PATCH] Switch to using a new HTTP client for each twirp client
 request
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ideally Crystal's HTTP client would manage a pool of connections and reuse them when appropriate, but it does not – see https://github.com/crystal-lang/crystal/issues/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.
---
 src/twirp/client.cr | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/twirp/client.cr b/src/twirp/client.cr
index c42e875..ab20a41 100644
--- a/src/twirp/client.cr
+++ b/src/twirp/client.cr
@@ -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?