TimonKK / clickhouse

NodeJS client for ClickHouse
Apache License 2.0
221 stars 122 forks source link

Issue with regex used to identify clickhouse error messages #104

Open mike-luabase opened 2 years ago

mike-luabase commented 2 years ago

The regex used to catch error messages from the clickhouse server is missing error codes with 3 digits e.g. error code 215. The change below would fix it, but I don't know clickhouse well enough to determine if this will cause other issues.

const R_ERROR = new RegExp('(Code|Error): .*Exception: (.+?)$', 'm');
mike-luabase commented 2 years ago

@TimonKK is there any reason we need to keep the ([0-9]{2})[,.] part of the regex?

mike-luabase commented 2 years ago

I see you updated here in September, can we drop the # check?

https://github.com/TimonKK/clickhouse/commit/a84d890a1d2d509f1377b1c9abf7da28307a42fc

TimonKK commented 2 years ago

What do you mean about "drop the # check"?

mike-luabase commented 2 years ago

@TimonKK the current regex is checking if there are numbers (([0-9]{2})[,.]) in the error code, is that necessary? If so, it'd need to check for at least three digits.

TimonKK commented 2 years ago

I know ClickHouse has two digital error codes, for example, https://github.com/ClickHouse/ClickHouse/issues/33839. So I think to adjust regexp to '(Code|Error): ([0-9]{2,})[,.] .*Exception: (.+?)$'

mike-luabase commented 2 years ago

@TimonKK The problem is there are now three digit error codes, e.g. I got 215

TimonKK commented 2 years ago

I understood about three-digit error codes. And I suggested new regexp /(Code|Error): ([0-9]{2,})[,.] .*Exception: (.+?)$/. You could test it on https://regexr.com/