No error forwarding when talking to clickhouse in "cluster mode"

[jeromefellus-sekoia]

How to reproduce

import httpx, asyncio, aiochclient

c = aiochclient.ChClient(httpx.AsyncClient(), url="http://localhost:8123")


async def main():
   # Errors responded by clickhouse in non-cluster mode are correctly forwarded as Exceptions
    try:
        res = await c.execute("CREATE TABLE x (a StupidType) ENGINE MergeTree()")
        print(res)
    except Exception as e:
        print("THIS ERROR IS CORRECTLY FORWARDED: ", e)

   # Errors in cluster mode are not raised, nor are they retrievable from the HTTP response... simply lost
    try:
        res = await c.execute("CREATE TABLE x ON CLUSTER mycluster (a StupidType) ENGINE MergeTree()")
        print(res) # Execute returns nothing, so you can't even retrieve the JSON-formatted error output
    except Exception as e:
        # We should get some exception...
        print("ERRORS FROM THE CLUSTER'S NODES ARE NOT FORWARDED, YOU'LL NEVER SEE THIS MESSAGE", e)

    await c.close()

asyncio.run(main())

Why it happens

• Clickhouse always responds with HTTP code 200, even if some node has failed to execute the statement. The error is embedded a JSON response like, e.g., :

{
	"meta":
	[
		{
			"name": "host",
			"type": "String"
		},
		{
			"name": "port",
			"type": "UInt16"
		},
		{
			"name": "status",
			"type": "Int64"
		},
		{
			"name": "error",
			"type": "String"
		},
		{
			"name": "num_hosts_remaining",
			"type": "UInt64"
		},
		{
			"name": "num_hosts_active",
			"type": "UInt64"
		}
	],

	"data":
	[
		["server101", 9000, "450", "Code: 450, e.displayText() = DB::Exception: TTL Expression GROUP BY key should be a prefix of primary key (version 21.3.20.1 (official build))", "5", "0"],
		["server2", 9000, "450", "Code: 450, e.displayText() = DB::Exception: TTL Expression GROUP BY key should be a prefix of primary key (version 21.3.20.1 (official build))", "4", "0"],
		["server103", 9000, "450", "Code: 450, e.displayText() = DB::Exception: TTL Expression GROUP BY key should be a prefix of primary key (version 21.3.20.1 (official build))", "3", "0"],
		["server3", 9000, "450", "Code: 450, e.displayText() = DB::Exception: TTL Expression GROUP BY key should be a prefix of primary key (version 21.3.20.1 (official build))", "2", "0"],
		["server102", 9000, "450", "Code: 450, e.displayText() = DB::Exception: TTL Expression GROUP BY key should be a prefix of primary key (version 21.3.20.1 (official build))", "1", "0"],
		["server1", 9000, "450", "Code: 450, e.displayText() = DB::Exception: TTL Expression GROUP BY key should be a prefix of primary key (version 21.3.20.1 (official build))", "0", "0"]

• We don't retrieve the HTTP response body for queries that went well (HTTP 200 code) in ChClient.execute => we don't have any access to those error messages embedded in the JSON response

Incriminated code

https://github.com/maximdanilchenko/aiochclient/blob/master/aiochclient/client.py#L189

we only exploit the HTTP response when we anticipate some records as a result. As CREATE TABLE and the like never return records, we simply drop the useful error messages when calling self._http_client.post_no_return(...).

Solution

Would you accept a PR that won't drop HTTP response body and when body embeds some error messages wrap the messages into some raised exception at ChClient._execute level ?

Is there any bad side effect to do this ?

[maximdanilchenko]

@jeromefellus-sekoia thank you for your issue!

For me your solution looks pretty good. So it would be great if you can make a PR!