Skip to content

Commit 811a7ce

Browse files
authored
fix(clickhouse-driver): Tune ClickHouse errors: remove SQL, catch ping failure (#8983)
* fix(clickhouse-driver): Remove SQL from ClickHouse error messages * fix(clickhouse-driver): Actually handle ping result * fix(clickhouse-driver): Handle AggregateError in ping result
1 parent eb33d1a commit 811a7ce

File tree

1 file changed

+13
-4
lines changed

1 file changed

+13
-4
lines changed

packages/cubejs-clickhouse-driver/src/ClickHouseDriver.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,16 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
185185
const { signal } = abortController;
186186

187187
const promise = (async () => {
188-
await this.client.ping();
188+
const pingResult = await this.client.ping();
189+
if (!pingResult.success) {
190+
// TODO replace string formatting with proper cause
191+
// pingResult.error can be AggregateError when ClickHouse hostname resolves to multiple addresses
192+
let errorMessage = pingResult.error.toString();
193+
if (pingResult.error instanceof AggregateError) {
194+
errorMessage = `Aggregate error: ${pingResult.error.message}; errors: ${pingResult.error.errors.join('; ')}`;
195+
}
196+
throw new Error(`Connection check failed: ${errorMessage}`);
197+
}
189198
signal.throwIfAborted();
190199
// Queries sent by `fn` can hit a timeout error, would _not_ get killed, and continue running in ClickHouse
191200
// TODO should we kill those as well?
@@ -260,7 +269,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
260269
return results;
261270
} catch (e) {
262271
// TODO replace string formatting with proper cause
263-
throw new Error(`Query failed; cause: ${e}; query id: ${queryId}; SQL: ${query}`);
272+
throw new Error(`Query failed: ${e}; query id: ${queryId}`);
264273
}
265274
});
266275
}
@@ -409,7 +418,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
409418
} catch (e) {
410419
await client.close();
411420
// TODO replace string formatting with proper cause
412-
throw new Error(`Stream query failed; cause: ${e}; query id: ${queryId}; SQL: ${query}`);
421+
throw new Error(`Stream query failed: ${e}; query id: ${queryId}`);
413422
}
414423
}
415424

@@ -554,7 +563,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
554563
await this.command(createTableSql);
555564
} catch (e) {
556565
// TODO replace string formatting with proper cause
557-
throw new Error(`Create table failed; cause: ${e}; SQL: ${createTableSql}`);
566+
throw new Error(`Create table failed: ${e}`);
558567
}
559568
}
560569

0 commit comments

Comments
 (0)