Skip to content

Commit

Permalink
Choose proper HTTP(S) request implementation based on the URL protocol (
Browse files Browse the repository at this point in the history
  • Loading branch information
slvrtrn authored Nov 14, 2024
1 parent a15cce9 commit 8b41e5b
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 10 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/client-common/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.8.0'
export default '1.8.1'
28 changes: 23 additions & 5 deletions packages/client-node/__tests__/tls/tls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
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(
'http_agent is required to create NodeCustomAgentConnection',
)
}
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 {
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/client-node/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.8.0'
export default '1.8.1'
2 changes: 1 addition & 1 deletion packages/client-web/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.8.0'
export default '1.8.1'

0 comments on commit 8b41e5b

Please sign in to comment.