refactor: update getClickHouseClient to be asynchronous and implement SSH tunneling

[Artuomka]

No description provided.

[Copilot]

Pull request overview

This PR refactors the ClickHouse data access object to support SSH tunneling by making the getClickHouseClient method asynchronous. The changes enable secure SSH tunnel connections to ClickHouse databases while maintaining backward compatibility with direct connections.

Key changes:

• Converted getClickHouseClient() from synchronous to asynchronous, updating all 16 call sites to use await
• Implemented SSH tunnel support with caching mechanism via createTunneledClickHouseClient()
• Extracted direct connection logic into createDirectClickHouseClient() for reuse

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

There's a race condition in the error handler registration. The Promise is resolved on line 866 before error handlers are registered on lines 868-876. If an error occurs immediately after the tunnel is established but before error handlers are attached, it will go unhandled. Additionally, if an error occurs after the promise has already been resolved, calling reject will have no effect since promises can only be settled once.

The error handlers should be registered before resolving the promise, and they should not call reject after the promise is resolved. Consider restructuring to match the pattern used in similar implementations where error handlers either don't reject after resolving, or the logic is structured to prevent this race condition.

[Copilot]

Using the Promise constructor with an async executor function is an anti-pattern. The executor function is marked as async but this is unnecessary and can lead to subtle bugs. The async keyword on the executor means it returns a Promise, but the Promise constructor doesn't handle this returned promise. If an error is thrown in an async executor before any await, it will create an unhandled promise rejection instead of being caught by the try-catch.

The async keyword should be removed from the executor function, or the code should be restructured to avoid the Promise constructor entirely by using async/await directly.

Suggested change

	return new Promise<ClickHouseClient>(async (resolve, reject): Promise<void> => {
	try {
	const [server, client] = await getTunnel(connectionCopy, freePort);
	const clickhouseClient = this.createDirectClickHouseClient('127.0.0.1', freePort);
	const tnlCachedObj = {
	server: server,
	client: client,
	clickhouse: clickhouseClient,
	};
	LRUStorage.setTunnelCache(connectionCopy, tnlCachedObj);
	resolve(clickhouseClient);
	client.on('error', (e) => {
	LRUStorage.delTunnelCache(connectionCopy);
	reject(e);
	});
	server.on('error', (e) => {
	LRUStorage.delTunnelCache(connectionCopy);
	reject(e);
	});
	} catch (error) {
	LRUStorage.delTunnelCache(connectionCopy);
	reject(error);
	}
	});
	try {
	const [server, client] = await getTunnel(connectionCopy, freePort);
	const clickhouseClient = this.createDirectClickHouseClient('127.0.0.1', freePort);
	const tnlCachedObj = {
	server: server,
	client: client,
	clickhouse: clickhouseClient,
	};
	LRUStorage.setTunnelCache(connectionCopy, tnlCachedObj);
	client.on('error', () => {
	LRUStorage.delTunnelCache(connectionCopy);
	});
	server.on('error', () => {
	LRUStorage.delTunnelCache(connectionCopy);
	});
	return clickhouseClient;
	} catch (error) {
	LRUStorage.delTunnelCache(connectionCopy);
	throw error;
	}

[Copilot]

The object property shorthand can be used here to improve code readability and reduce redundancy. Instead of explicitly assigning 'server: server' and 'client: client', these can be simplified using shorthand property syntax.

Suggested change

	server: server,
	client: client,
	server,
	client,