Skip to content

Commit 86f4d59

Browse files
cvclaude
andcommitted
Fix exception handling in select_all to raise errors instead of returning empty results
Fixes #50 When ClickHouse returns error responses with an 'exception' field in the JSON body (but without HTTP error codes), the Factory.response method was silently treating these as empty result sets instead of raising exceptions. This caused invalid queries to return [] instead of raising DbException. Changes: - Add exception detection in Response::Factory.response before processing body - Raise DbException when response body contains 'exception' field - Add comprehensive unit tests for Factory.response with error scenarios - Add integration tests for select_all, select_one, and select_value error handling All 143 tests pass with no regressions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 663b72d commit 86f4d59

File tree

3 files changed

+86
-0
lines changed

3 files changed

+86
-0
lines changed

lib/click_house/response/factory.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ class Factory
1212
def self.response(faraday, config)
1313
body = faraday.body
1414

15+
# Check for exception in response body (ClickHouse may return errors with HTTP 200)
16+
# This handles cases where the RaiseError middleware doesn't catch the error
17+
if body.is_a?(Hash) && body.key?('exception')
18+
raise DbException, body['exception']
19+
end
20+
1521
# wrap to be able to use connection#select_one, connection#select_value
1622
# with other formats like binary
1723
return raw(faraday, config) unless body.is_a?(Hash)

spec/click_house/extend/connection_selective_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# frozen_string_literal: true
2+
13
RSpec.describe ClickHouse::Extend::ConnectionSelective do
24
subject do
35
ClickHouse.connection
@@ -21,6 +23,12 @@
2123
expect(subject.select_value('SELECT 1, 2, 3, 4, 5')).to eq(1)
2224
end
2325
end
26+
27+
context 'when invalid query' do
28+
it 'raises DbException for unknown function' do
29+
expect { subject.select_value('SELECT unknownFunction()') }.to raise_error(ClickHouse::DbException)
30+
end
31+
end
2432
end
2533

2634
describe '#select_one' do
@@ -35,6 +43,12 @@
3543
expect(subject.select_one('SELECT NULL')).to eq({ 'NULL' => nil })
3644
end
3745
end
46+
47+
context 'when invalid query' do
48+
it 'raises DbException for unknown function' do
49+
expect { subject.select_one('SELECT unknownFunction()') }.to raise_error(ClickHouse::DbException)
50+
end
51+
end
3852
end
3953

4054
describe '#select_all' do
@@ -66,5 +80,19 @@
6680
expect(subject.select_all('SELECT * FROM rspec').to_a).to match_array(expectation)
6781
end
6882
end
83+
84+
context 'when invalid query' do
85+
it 'raises DbException for unknown function' do
86+
expect { subject.select_all('SELECT unknownFunction()') }.to raise_error(ClickHouse::DbException)
87+
end
88+
89+
it 'raises DbException for non-existent table' do
90+
expect { subject.select_all('SELECT * FROM nonexistent_table_xyz') }.to raise_error(ClickHouse::DbException)
91+
end
92+
93+
it 'raises DbException with error message for invalid syntax' do
94+
expect { subject.select_all('SELECT * FROM') }.to raise_error(ClickHouse::DbException, /Exception/)
95+
end
96+
end
6997
end
7098
end

spec/click_house/response/factory_spec.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,60 @@
1+
# frozen_string_literal: true
2+
13
RSpec.describe ClickHouse::Response::Factory do
24
subject do
35
ClickHouse.connection
46
end
57

8+
describe '.response' do
9+
let(:config) { ClickHouse::Config.new }
10+
let(:faraday_env) { {} }
11+
let(:summary_middleware) { instance_double(ClickHouse::Middleware::SummaryMiddleware) }
12+
13+
before do
14+
allow(ClickHouse::Middleware::SummaryMiddleware).to receive(:extract).and_return(double('summary'))
15+
end
16+
17+
context 'when response contains exception field' do
18+
let(:faraday_response) do
19+
double('Faraday::Response',
20+
body: { 'exception' => 'Code: 46. DB::Exception: Unknown function' },
21+
env: faraday_env)
22+
end
23+
24+
it 'raises DbException' do
25+
expect do
26+
ClickHouse::Response::Factory.response(faraday_response, config)
27+
end.to raise_error(ClickHouse::DbException, /Unknown function/)
28+
end
29+
end
30+
31+
context 'when response contains valid data' do
32+
let(:faraday_response) do
33+
double('Faraday::Response',
34+
body: { 'meta' => [], 'data' => [] },
35+
env: faraday_env)
36+
end
37+
38+
it 'returns ResultSet' do
39+
result = ClickHouse::Response::Factory.response(faraday_response, config)
40+
expect(result).to be_a(ClickHouse::Response::ResultSet)
41+
end
42+
end
43+
44+
context 'when response is not a Hash' do
45+
let(:faraday_response) do
46+
double('Faraday::Response',
47+
body: 'plain text response',
48+
env: faraday_env)
49+
end
50+
51+
it 'returns raw ResultSet' do
52+
result = ClickHouse::Response::Factory.response(faraday_response, config)
53+
expect(result).to be_a(ClickHouse::Response::ResultSet)
54+
end
55+
end
56+
end
57+
658
describe 'WITH totals modifier' do
759
context 'when blank' do
860
let(:response) do

0 commit comments

Comments
 (0)