Skip to content

Conversation

@cv
Copy link

@cv cv commented Oct 24, 2025

Summary

Fixes #50 - select_all and related methods now properly raise DbException when ClickHouse returns error responses, instead of silently returning empty results.

Problem

When ClickHouse returns error responses with an 'exception' field in the JSON body (but without HTTP error codes or exception headers), the Response::Factory.response method was silently treating these as empty result sets instead of raising exceptions. This caused invalid queries like SELECT unknownFunction() to return [] instead of raising a proper error.

Solution

Added exception detection in Response::Factory.response before processing the response body. When the body is a Hash containing an 'exception' key, it now raises DbException with the error message from ClickHouse.

Key changes:

  • lib/click_house/response/factory.rb - Added 4 lines to check for exception field and raise DbException
  • Added comprehensive unit tests for Factory.response with error scenarios
  • Added integration tests for select_all, select_one, and select_value error handling

Testing

  • All 143 existing tests pass with no regressions ✅
  • Added 5 new unit tests for Factory.response error handling ✅
  • Added 5 new integration tests for query method error handling ✅
  • Tested with Docker using ClickHouse 22.9 ✅
  • RuboCop compliant ✅

Code Quality

  • Minimal code change (4 lines)
  • Follows existing error handling patterns (uses DbException)
  • Well-documented with inline comments
  • Consistent with codebase architecture
  • No breaking changes (backward compatible)

🤖 Generated with Claude Code

…ning empty results

Fixes shlima#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

select_all swallows exceptions for invalid queries or server errors

1 participant