clickhouse-go icon indicating copy to clipboard operation
clickhouse-go copied to clipboard

Inconsistent error types returned between the `Native` and `HTTP` protocols.

Open r0bobo opened this issue 3 months ago • 3 comments

Observed

The returned errors are of different types when switching between the Native and HTTP protocols. We have a large codebase that uses the Native protocol but are trying to switch to the HTTP protocol to be able to use HTTP load balancers in front of ClickHouse. After switching to HTTP protocol error assertions don't catch errors any more since the underlying error type has changed.

Expected behaviour

Same error types returned from both HTTP and Native protocol.

Code example

package main

import (
	"errors"
	"testing"

	"github.com/ClickHouse/clickhouse-go/v2"
)

func TestErrors(t *testing.T) {
	for _, tt := range []struct {
		port     string
		protocol clickhouse.Protocol
	}{
		{
			protocol: clickhouse.Native,
			port:     "9000",
		},
		{
			protocol: clickhouse.HTTP,
			port:     "8123",
		},
	} {
		t.Run(tt.protocol.String(), func(t *testing.T) {
			db := clickhouse.OpenDB(&clickhouse.Options{
				Addr:     []string{"localhost:" + tt.port},
				Protocol: tt.protocol,
			})
			_, err := db.Query("SELECT * FROM non_existent_table")
			var chErr *clickhouse.Exception
			if errors.As(err, &chErr) {
				if chErr.Code != 60 {
					t.Errorf("unknown error: %s", err.Error())

				}
			} else if err != nil {
				t.Errorf("unknown error: %s", err.Error())
			}
		})
	}
}

Error log

❯ go test ./test/...
--- FAIL: TestErrors (0.00s)
    --- FAIL: TestErrors/http (0.00s)
        foo_test.go:37: unknown error: sendQuery: [HTTP 404] response body: "Code: 60. DB::Exception: Unknown table expression identifier 'non_existent_table' in scope SELECT * FROM non_existent_table. (UNKNOWN_TABLE) (version 25.1.5.31 (official build))
            "
FAIL
FAIL    github.com/formulatehq/ingest/test      0.009s
FAIL

Details

Environment

  • [x] clickhouse-go version: v2.40.3
  • [x] Interface: database/sql compatible driver
  • [x] Go version: go1.25.0 linux/amd64
  • [x] Operating system: Fedora 42
  • [x] ClickHouse version: 25.1.5.31
  • [x] Is it a ClickHouse Cloud? No
  • [ ] ClickHouse Server non-default settings, if any:
  • [ ] CREATE TABLE statements for tables involved:
  • [ ] Sample data for all these tables, use clickhouse-obfuscator if necessary

r0bobo avatar Sep 26 '25 07:09 r0bobo

The behavior is due to the fact, error values and it's types not same with TCP and HTTP currently.

For example this HTTP error in above example is simple ac-hoc error created on the fly.

And the api errors.As() will succeed only if any of the wrapped errors in target matches the source error type.

Having the ability to have this kind of flexibility needs some refactoring. May be @SpencerTorres have some thoughts here?

@r0bobo Also out of curiosity, what is your use case of switching from TCP -> HTTP? is it related to any features in HTTP? or anything else?

kavirajk avatar Sep 29 '25 11:09 kavirajk

@r0bobo Also out of curiosity, what is your use case of switching from TCP -> HTTP? is it related to any features in HTTP? or anything else?

We want to route requests to different clusters based on things such as tenant id and with http protocol we can just use standard http proxies and headers.

r0bobo avatar Sep 30 '25 06:09 r0bobo

Thanks @r0bobo, that make sense to use HTTP :)

The main problem is Native (TCP) and HTTP errors can be completely different and may not make sense to unify. e.g: HTTP status codes has no meaning with TCP.

Technically, we can have common error type something like clickhouse.Error interface that will be implemented by any error from both HTTP and TCP, but when it comes to usage, it would still be tricky, because user cannot do much with that error (other than just using errors.As() to check) because user still would have to convert those common error to more concrete error to interpret and do something useful (e.g: check what status code is if it's HTTP).

I'm also curious, if you trying to migrate from TCP -> HTTP without needing much code changes on error handling? Is that the goal? Because it makes sense to change the error handling code as well when migrating it from TCP -> HTTP because of the differences with both transport errors IMHO.

kavirajk avatar Sep 30 '25 08:09 kavirajk