From b3a011590bd44b53736e41a73cdcf642ac3621f7 Mon Sep 17 00:00:00 2001
From: Vincent Bray <vincent.bray@sage.com>
Date: Tue, 25 Apr 2023 10:29:32 +0000
Subject: [PATCH 1/3] Avoid adding nil to connection pool on error

---
 lib/mysql_framework/connector.rb           |  2 +-
 spec/lib/mysql_framework/connector_spec.rb | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/mysql_framework/connector.rb b/lib/mysql_framework/connector.rb
index d3ac319..ee54d32 100644
--- a/lib/mysql_framework/connector.rb
+++ b/lib/mysql_framework/connector.rb
@@ -77,7 +77,7 @@ def with_client(provided = nil)
       client = provided || check_out
       yield client
     ensure
-      check_in(client) unless provided
+      check_in(client) if client && !provided
     end
 
     # This method is called to execute a prepared statement
diff --git a/spec/lib/mysql_framework/connector_spec.rb b/spec/lib/mysql_framework/connector_spec.rb
index f50c147..c38fd5e 100644
--- a/spec/lib/mysql_framework/connector_spec.rb
+++ b/spec/lib/mysql_framework/connector_spec.rb
@@ -425,4 +425,14 @@
       end
     end
   end
+
+  describe 'connection pool error handling' do
+    it 'does not add nil to the pool when full and an attempt is made to use it' do
+      conns = max_pool_size.times.map { subject.check_out }
+      expect { subject.query('select 1') }.to raise_error(RuntimeError)
+      # Previous versions had a bug where a nil would be added to the pool
+      # at this point.
+      expect { subject.connections.pop(true) } .to raise_error(ThreadError)
+    end
+  end
 end

From 51065504d10b9f2e542b02994ceb21e6a6889b73 Mon Sep 17 00:00:00 2001
From: Vincent Bray <vincent.bray@sage.com>
Date: Tue, 25 Apr 2023 10:42:46 +0000
Subject: [PATCH 2/3] Verify check_in not called

---
 spec/lib/mysql_framework/connector_spec.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/spec/lib/mysql_framework/connector_spec.rb b/spec/lib/mysql_framework/connector_spec.rb
index c38fd5e..d652478 100644
--- a/spec/lib/mysql_framework/connector_spec.rb
+++ b/spec/lib/mysql_framework/connector_spec.rb
@@ -427,6 +427,10 @@
   end
 
   describe 'connection pool error handling' do
+    before do
+      expect(subject).to_not receive(:check_in)
+    end
+
     it 'does not add nil to the pool when full and an attempt is made to use it' do
       conns = max_pool_size.times.map { subject.check_out }
       expect { subject.query('select 1') }.to raise_error(RuntimeError)

From e7678931b71e1dbab0436a8a067e9db8660b91fa Mon Sep 17 00:00:00 2001
From: Vincent Bray <vincent.bray@sage.com>
Date: Tue, 25 Apr 2023 11:08:37 +0000
Subject: [PATCH 3/3] Expand connection pool error handling specs

---
 spec/lib/mysql_framework/connector_spec.rb | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/spec/lib/mysql_framework/connector_spec.rb b/spec/lib/mysql_framework/connector_spec.rb
index d652478..53364d0 100644
--- a/spec/lib/mysql_framework/connector_spec.rb
+++ b/spec/lib/mysql_framework/connector_spec.rb
@@ -426,17 +426,22 @@
     end
   end
 
-  describe 'connection pool error handling' do
+  describe 'when connection pool is exhausted' do
     before do
-      expect(subject).to_not receive(:check_in)
+      max_pool_size.times { subject.check_out }
     end
 
-    it 'does not add nil to the pool when full and an attempt is made to use it' do
-      conns = max_pool_size.times.map { subject.check_out }
-      expect { subject.query('select 1') }.to raise_error(RuntimeError)
-      # Previous versions had a bug where a nil would be added to the pool
-      # at this point.
-      expect { subject.connections.pop(true) } .to raise_error(ThreadError)
+    it 'pop throws exception' do
+      expect { subject.connections.pop(true) }.to raise_error(ThreadError)
+    end
+
+    it 'throws exception on query' do
+      expect { subject.query('SELECT 1') }.to raise_error(RuntimeError, /depleted/)
+    end
+
+    it 'does not put nil in the pool on error' do
+      expect(subject).to_not receive(:check_in).with(nil)
+      expect { subject.query('SELECT 1') }.to raise_error(RuntimeError, /depleted/)
     end
   end
 end