From adf4c5268815dddeefca83bab0988bdd007df47c Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Thu, 4 Jun 2026 15:45:42 -0700 Subject: [PATCH 1/8] json_request raises ConnectionError --- lib/elastic_record/connection.rb | 2 ++ test/elastic_record/connection_test.rb | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/elastic_record/connection.rb b/lib/elastic_record/connection.rb index d5b753d..6459cd7 100644 --- a/lib/elastic_record/connection.rb +++ b/lib/elastic_record/connection.rb @@ -42,6 +42,8 @@ def json_request(method, path, payload) response = http_request_with_retry(method, path, payload) + raise_connection_error(response, payload) unless (200..299).cover?(response.code.to_i) + response_body = ActiveSupport::JSON.decode(response.body) response_body['error'] ? raise_connection_error(response, payload) : response_body end diff --git a/test/elastic_record/connection_test.rb b/test/elastic_record/connection_test.rb index 4ba2045..a158bad 100644 --- a/test/elastic_record/connection_test.rb +++ b/test/elastic_record/connection_test.rb @@ -30,6 +30,17 @@ def test_json_requests assert_equal expected, connection.json_put("/test") end + def test_json_request_400_errors + stub_es_request(:get, "/test").to_return(status: 413, body: 'client intended to send too large a body') + + error = + assert_raises ElasticRecord::ConnectionError do + connection.json_get("/test") + end + + assert_equal '413', error.status_code + end + def test_json_request_with_valid_error_status request = { 'some' => 'payload' } response = { 'error' => 'Doing it wrong' } From d4a386cb3b0b5f92c49cd75c712a1b3679a0639f Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Thu, 4 Jun 2026 18:15:24 -0700 Subject: [PATCH 2/8] 'get' and 'delete' can 404 --- lib/elastic_record/connection.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/elastic_record/connection.rb b/lib/elastic_record/connection.rb index 6459cd7..57e239b 100644 --- a/lib/elastic_record/connection.rb +++ b/lib/elastic_record/connection.rb @@ -22,7 +22,7 @@ def head(path) end def json_get(path, json = nil) - json_request :get, path, json + json_request :get, path, json, consider_ok: 404 end def json_post(path, json = nil) @@ -34,15 +34,19 @@ def json_put(path, json = nil) end def json_delete(path, json = nil) - json_request :delete, path, json + json_request :delete, path, json, consider_ok: 404 end - def json_request(method, path, payload) + def json_request(method, path, payload, consider_ok: []) payload = ActiveSupport::JSON.encode(payload) if payload.is_a?(Hash) response = http_request_with_retry(method, path, payload) - raise_connection_error(response, payload) unless (200..299).cover?(response.code.to_i) + code = response.code.to_i + + unless (200..299).cover?(code) || Array.wrap(consider_ok).include?(code) + raise_connection_error(response, payload) + end response_body = ActiveSupport::JSON.decode(response.body) response_body['error'] ? raise_connection_error(response, payload) : response_body From 9ee5b7b0dbefa77e10fa605ebce437e10a5f2d93 Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Fri, 5 Jun 2026 15:12:43 -0700 Subject: [PATCH 3/8] consolidate error handling into http_request_with_retry --- lib/elastic_record/connection.rb | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/elastic_record/connection.rb b/lib/elastic_record/connection.rb index 57e239b..8a5b05d 100644 --- a/lib/elastic_record/connection.rb +++ b/lib/elastic_record/connection.rb @@ -18,11 +18,11 @@ def initialize(servers, options = {}) end def head(path) - http_request_with_retry(:head, path).code + http_request_with_retry(:head, path, nil, [404]).code end def json_get(path, json = nil) - json_request :get, path, json, consider_ok: 404 + json_request :get, path, json, ok_statuses: [404] end def json_post(path, json = nil) @@ -34,31 +34,30 @@ def json_put(path, json = nil) end def json_delete(path, json = nil) - json_request :delete, path, json, consider_ok: 404 + json_request :delete, path, json, ok_statuses: [404] end - def json_request(method, path, payload, consider_ok: []) + def json_request(method, path, payload, ok_statuses: []) payload = ActiveSupport::JSON.encode(payload) if payload.is_a?(Hash) - response = http_request_with_retry(method, path, payload) - - code = response.code.to_i - - unless (200..299).cover?(code) || Array.wrap(consider_ok).include?(code) - raise_connection_error(response, payload) - end + response = http_request_with_retry(method, path, payload, ok_statuses) response_body = ActiveSupport::JSON.decode(response.body) response_body['error'] ? raise_connection_error(response, payload) : response_body end - def http_request_with_retry(method, path, payload = nil) + STATUS_200 = (200..299).freeze + + def http_request_with_retry(method, path, payload = nil, ok_statuses = []) with_retry do response = http_request(method, path, payload) + code = response.code.to_i - raise_connection_error(response, payload) if response.code.to_i >= 500 - - response + if STATUS_200.cover?(code) || ok_statuses.include?(code) + response + else + raise_connection_error(response, payload) + end end end From 041435a455b19f55ab0b9ae5bc7bf093cf8967f1 Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Fri, 5 Jun 2026 16:20:57 -0700 Subject: [PATCH 4/8] add a test for json_get 404 --- test/elastic_record/connection_test.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/elastic_record/connection_test.rb b/test/elastic_record/connection_test.rb index a158bad..5a1801c 100644 --- a/test/elastic_record/connection_test.rb +++ b/test/elastic_record/connection_test.rb @@ -30,7 +30,7 @@ def test_json_requests assert_equal expected, connection.json_put("/test") end - def test_json_request_400_errors + def test_json_get_413_error stub_es_request(:get, "/test").to_return(status: 413, body: 'client intended to send too large a body') error = @@ -41,6 +41,23 @@ def test_json_request_400_errors assert_equal '413', error.status_code end + def test_json_get_404_error + response = { error: { type: "index_not_found_exception" } } + + stub_es_request(:get, "/test").to_return(status: 404, body: response.to_json) + + error = + assert_raises ElasticRecord::ConnectionError do + connection.json_get("/test") + end + + expected = { + elasticsearch_response: response, + request_payload: nil, + }.to_json + assert_equal expected, error.message + end + def test_json_request_with_valid_error_status request = { 'some' => 'payload' } response = { 'error' => 'Doing it wrong' } From eae05be9e46e99e6f46b78bd7102bfceba8c2ff2 Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Fri, 5 Jun 2026 16:22:11 -0700 Subject: [PATCH 5/8] single quotes --- test/elastic_record/connection_test.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/elastic_record/connection_test.rb b/test/elastic_record/connection_test.rb index 5a1801c..2debdc5 100644 --- a/test/elastic_record/connection_test.rb +++ b/test/elastic_record/connection_test.rb @@ -31,24 +31,24 @@ def test_json_requests end def test_json_get_413_error - stub_es_request(:get, "/test").to_return(status: 413, body: 'client intended to send too large a body') + stub_es_request(:get, '/test').to_return(status: 413, body: 'client intended to send too large a body') error = assert_raises ElasticRecord::ConnectionError do - connection.json_get("/test") + connection.json_get('/test') end assert_equal '413', error.status_code end def test_json_get_404_error - response = { error: { type: "index_not_found_exception" } } + response = { error: { type: 'index_not_found_exception' } } - stub_es_request(:get, "/test").to_return(status: 404, body: response.to_json) + stub_es_request(:get, '/test').to_return(status: 404, body: response.to_json) error = assert_raises ElasticRecord::ConnectionError do - connection.json_get("/test") + connection.json_get('/test') end expected = { From f182ab2b5b7b35de81b98b65923fb4b37704993e Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Fri, 5 Jun 2026 16:59:03 -0700 Subject: [PATCH 6/8] test 404 without error in response body --- test/elastic_record/connection_test.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/test/elastic_record/connection_test.rb b/test/elastic_record/connection_test.rb index 2debdc5..c579f8e 100644 --- a/test/elastic_record/connection_test.rb +++ b/test/elastic_record/connection_test.rb @@ -30,7 +30,7 @@ def test_json_requests assert_equal expected, connection.json_put("/test") end - def test_json_get_413_error + def test_json_get_413_no_error stub_es_request(:get, '/test').to_return(status: 413, body: 'client intended to send too large a body') error = @@ -41,7 +41,7 @@ def test_json_get_413_error assert_equal '413', error.status_code end - def test_json_get_404_error + def test_json_get_404_with_error response = { error: { type: 'index_not_found_exception' } } stub_es_request(:get, '/test').to_return(status: 404, body: response.to_json) @@ -58,6 +58,21 @@ def test_json_get_404_error assert_equal expected, error.message end + def test_json_get_404_without_error + response = { '_index' => 'places', '_id' => '123', 'found' => false } + + stub_es_request(:get, '/places/_doc/123').to_return(status: 404, body: response.to_json) + + result = + begin + connection.json_get('/places/_doc/123') + rescue + raise StandardError.new('"get" requests without error bodies should not raise!') + end + + assert_equal response, result + end + def test_json_request_with_valid_error_status request = { 'some' => 'payload' } response = { 'error' => 'Doing it wrong' } From 0e5a3d9e892756ed8fc2f69e73f98b1179d9b9a1 Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Fri, 5 Jun 2026 17:13:49 -0700 Subject: [PATCH 7/8] add test for 404 delete w/o error --- test/elastic_record/connection_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/elastic_record/connection_test.rb b/test/elastic_record/connection_test.rb index c579f8e..3e3b6f8 100644 --- a/test/elastic_record/connection_test.rb +++ b/test/elastic_record/connection_test.rb @@ -73,6 +73,17 @@ def test_json_get_404_without_error assert_equal response, result end + def test_json_delete_404_without_error + request = { 'id' => 'expired_pit' } + response = { 'succeeded' => false, 'num_freed' => 0 } + + stub_es_request(:delete, '/_pit').to_return(status: 404, body: response.to_json) + + result = connection.json_delete('/_pit', request.to_json) + + assert_equal response, result + end + def test_json_request_with_valid_error_status request = { 'some' => 'payload' } response = { 'error' => 'Doing it wrong' } From 3b6455ba4caf6c23521d5d251bb5abfb3688b281 Mon Sep 17 00:00:00 2001 From: Matthew Hinea Date: Fri, 5 Jun 2026 17:16:08 -0700 Subject: [PATCH 8/8] use 'flunk' for better error messaging --- test/elastic_record/connection_test.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/elastic_record/connection_test.rb b/test/elastic_record/connection_test.rb index 3e3b6f8..7dd3f6b 100644 --- a/test/elastic_record/connection_test.rb +++ b/test/elastic_record/connection_test.rb @@ -66,8 +66,8 @@ def test_json_get_404_without_error result = begin connection.json_get('/places/_doc/123') - rescue - raise StandardError.new('"get" requests without error bodies should not raise!') + rescue ElasticRecord::ConnectionError + flunk('404 GET without error should not raise') end assert_equal response, result @@ -79,7 +79,12 @@ def test_json_delete_404_without_error stub_es_request(:delete, '/_pit').to_return(status: 404, body: response.to_json) - result = connection.json_delete('/_pit', request.to_json) + result = + begin + connection.json_delete('/_pit', request.to_json) + rescue ElasticRecord::ConnectionError + flunk('404 DELETE without error should not raise') + end assert_equal response, result end