From 8b41e5b5916a84f8bf0aa422af58709442b661cc Mon Sep 17 00:00:00 2001
From: Serge Klochkov <3175289+slvrtrn@users.noreply.github.com>
Date: Thu, 14 Nov 2024 19:04:46 +0100
Subject: [PATCH] Choose proper HTTP(S) request implementation based on the URL
 protocol (#354)

---
 CHANGELOG.md                                  |  6 ++++
 packages/client-common/src/version.ts         |  2 +-
 .../client-node/__tests__/tls/tls.test.ts     | 28 +++++++++++++++----
 .../node_custom_agent_connection.ts           | 13 +++++++--
 packages/client-node/src/version.ts           |  2 +-
 packages/client-web/src/version.ts            |  2 +-
 6 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2203b42..3cdff47b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 1.8.1 (Node.js)
+
+## Bug fixes
+
+- When a custom HTTP agent is used, the HTTP or HTTPS request implementation is now correctly chosen based on the URL protocol. ([#352](https://github.com/ClickHouse/clickhouse-js/issues/352))
+
 # 1.8.0 (Common, Node.js, Web)
 
 ## New features
diff --git a/packages/client-common/src/version.ts b/packages/client-common/src/version.ts
index 5507b15a..e409cba9 100644
--- a/packages/client-common/src/version.ts
+++ b/packages/client-common/src/version.ts
@@ -1 +1 @@
-export default '1.8.0'
+export default '1.8.1'
diff --git a/packages/client-node/__tests__/tls/tls.test.ts b/packages/client-node/__tests__/tls/tls.test.ts
index 2bd0be73..a2d5992e 100644
--- a/packages/client-node/__tests__/tls/tls.test.ts
+++ b/packages/client-node/__tests__/tls/tls.test.ts
@@ -5,6 +5,8 @@ import Http from 'http'
 import https from 'node:https'
 import type Stream from 'stream'
 import { createClient } from '../../src'
+import Https from 'https'
+import http from 'http'
 
 describe('[Node.js] TLS connection', () => {
   let client: ClickHouseClient<Stream.Readable>
@@ -139,12 +141,8 @@ describe('[Node.js] TLS connection', () => {
     })
 
     describe('Custom HTTPS agent', () => {
-      let httpRequestStub: jasmine.Spy<typeof Http.request>
-      beforeEach(() => {
-        httpRequestStub = spyOn(Http, 'request').and.callThrough()
-      })
-
       it('should work with a custom HTTPS agent', async () => {
+        const httpsRequestStub = spyOn(Https, 'request').and.callThrough()
         const agent = new https.Agent({
           maxFreeSockets: 5,
           ca: ca_cert,
@@ -163,6 +161,26 @@ describe('[Node.js] TLS connection', () => {
           format: 'JSONEachRow',
         })
         expect(await rs.json()).toEqual([{ result: 144 }])
+        expect(httpsRequestStub).toHaveBeenCalledTimes(1)
+        const callArgs = httpsRequestStub.calls.mostRecent().args
+        expect(callArgs[1].agent).toBe(agent)
+      })
+
+      // does not really belong to the TLS test; keep it here for consistency
+      it('should work with a custom HTTP agent', async () => {
+        const httpRequestStub = spyOn(Http, 'request').and.callThrough()
+        const agent = new http.Agent({
+          maxFreeSockets: 5,
+        })
+        const client = createClient({
+          url: 'http://localhost:8123',
+          http_agent: agent,
+        })
+        const rs = await client.query({
+          query: 'SELECT 144 AS result',
+          format: 'JSONEachRow',
+        })
+        expect(await rs.json()).toEqual([{ result: 144 }])
         expect(httpRequestStub).toHaveBeenCalledTimes(1)
         const callArgs = httpRequestStub.calls.mostRecent().args
         expect(callArgs[1].agent).toBe(agent)
diff --git a/packages/client-node/src/connection/node_custom_agent_connection.ts b/packages/client-node/src/connection/node_custom_agent_connection.ts
index 15d85f61..8e27fb91 100644
--- a/packages/client-node/src/connection/node_custom_agent_connection.ts
+++ b/packages/client-node/src/connection/node_custom_agent_connection.ts
@@ -1,12 +1,14 @@
-import { withCompressionHeaders } from '@clickhouse/client-common'
 import Http from 'http'
+import Https from 'https'
 import type {
   NodeConnectionParams,
   RequestParams,
 } from './node_base_connection'
 import { NodeBaseConnection } from './node_base_connection'
+import { withCompressionHeaders } from '@clickhouse/client-common'
 
 export class NodeCustomAgentConnection extends NodeBaseConnection {
+  private readonly httpRequestFn: typeof Http.request | typeof Https.request
   constructor(params: NodeConnectionParams) {
     if (!params.http_agent) {
       throw new Error(
@@ -14,6 +16,13 @@ export class NodeCustomAgentConnection extends NodeBaseConnection {
       )
     }
     super(params, params.http_agent)
+
+    // See https://github.com/ClickHouse/clickhouse-js/issues/352
+    if (params.url.protocol.startsWith('https')) {
+      this.httpRequestFn = Https.request
+    } else {
+      this.httpRequestFn = Http.request
+    }
   }
 
   protected createClientRequest(params: RequestParams): Http.ClientRequest {
@@ -22,7 +31,7 @@ export class NodeCustomAgentConnection extends NodeBaseConnection {
       enable_request_compression: params.enable_request_compression,
       enable_response_compression: params.enable_response_compression,
     })
-    return Http.request(params.url, {
+    return this.httpRequestFn(params.url, {
       method: params.method,
       agent: this.agent,
       timeout: this.params.request_timeout,
diff --git a/packages/client-node/src/version.ts b/packages/client-node/src/version.ts
index 5507b15a..e409cba9 100644
--- a/packages/client-node/src/version.ts
+++ b/packages/client-node/src/version.ts
@@ -1 +1 @@
-export default '1.8.0'
+export default '1.8.1'
diff --git a/packages/client-web/src/version.ts b/packages/client-web/src/version.ts
index 5507b15a..e409cba9 100644
--- a/packages/client-web/src/version.ts
+++ b/packages/client-web/src/version.ts
@@ -1 +1 @@
-export default '1.8.0'
+export default '1.8.1'