From 80fec2c7d7baff084221539697512be65cf866ea Mon Sep 17 00:00:00 2001 From: Sebastian Lorenz Date: Mon, 3 Aug 2026 05:59:53 +0000 Subject: [PATCH 1/2] Add reproduction for clickhouse/ClickhouseClient issue --- .../test/ConnectTimeoutLeakRepro.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts diff --git a/packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts b/packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts new file mode 100644 index 00000000000..ac7a4d99a7e --- /dev/null +++ b/packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts @@ -0,0 +1,32 @@ +import { ClickhouseClient } from "@effect/sql-clickhouse" +import { assert, it } from "@effect/vitest" +import { Effect } from "effect" +import { TestClock } from "effect/testing" +import * as Reactivity from "effect/unstable/reactivity/Reactivity" +import { vi } from "vitest" + +let closeCalls = 0 + +vi.mock("@clickhouse/client", () => ({ + createClient: () => ({ + exec: () => new Promise(() => {}), + close: () => { + closeCalls++ + return Promise.resolve() + } + }) +})) + +it.effect("closes a ClickHouse client when its connection check times out", () => + Effect.gen(function*() { + closeCalls = 0 + const fiber = yield* Effect.forkDetach( + ClickhouseClient.make({ url: "http://localhost:8123" }).pipe(Effect.scoped) + ) + yield* Effect.yieldNow + yield* TestClock.adjust("5 seconds") + const result = fiber.pollUnsafe() + + assert.isDefined(result) + assert.strictEqual(closeCalls, 1) + }).pipe(Effect.provide(Reactivity.layer))) From 91f1135e64a048019aeb426aa0208e0279ce347b Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Tue, 4 Aug 2026 09:22:51 +1200 Subject: [PATCH 2/2] Fix ClickHouse connect timeout cleanup --- .changeset/fix-clickhouse-connect-timeout.md | 5 +++ .../sql/clickhouse/src/ClickhouseClient.ts | 18 +++++------ packages/sql/clickhouse/test/Client.test.ts | 29 +++++++++++++++++ .../test/ConnectTimeoutLeakRepro.test.ts | 32 ------------------- 4 files changed, 43 insertions(+), 41 deletions(-) create mode 100644 .changeset/fix-clickhouse-connect-timeout.md delete mode 100644 packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts diff --git a/.changeset/fix-clickhouse-connect-timeout.md b/.changeset/fix-clickhouse-connect-timeout.md new file mode 100644 index 00000000000..41a80e36907 --- /dev/null +++ b/.changeset/fix-clickhouse-connect-timeout.md @@ -0,0 +1,5 @@ +--- +"@effect/sql-clickhouse": patch +--- + +Close the ClickHouse client when the startup connection check times out. diff --git a/packages/sql/clickhouse/src/ClickhouseClient.ts b/packages/sql/clickhouse/src/ClickhouseClient.ts index 184881913d8..a11a851a77e 100644 --- a/packages/sql/clickhouse/src/ClickhouseClient.ts +++ b/packages/sql/clickhouse/src/ClickhouseClient.ts @@ -175,16 +175,16 @@ export const make = ( ? Statement.defaultTransforms(options.transformResultNames).array : undefined - const client = Clickhouse.createClient(options) + const client = yield* Effect.acquireRelease( + Effect.sync(() => Clickhouse.createClient(options)), + (client) => Effect.promise(() => client.close()) + ) - yield* Effect.acquireRelease( - Effect.tryPromise({ - try: () => client.exec({ query: "SELECT 1" }), - catch: (cause) => - new SqlError({ reason: classifyError(cause, "ClickhouseClient: Failed to connect", "connect", "connection") }) - }), - () => Effect.promise(() => client.close()) - ).pipe( + yield* Effect.tryPromise({ + try: () => client.exec({ query: "SELECT 1" }), + catch: (cause) => + new SqlError({ reason: classifyError(cause, "ClickhouseClient: Failed to connect", "connect", "connection") }) + }).pipe( Effect.timeoutOrElse({ duration: Duration.seconds(5), orElse: () => diff --git a/packages/sql/clickhouse/test/Client.test.ts b/packages/sql/clickhouse/test/Client.test.ts index 5cb6deea5a1..44c52bda55d 100644 --- a/packages/sql/clickhouse/test/Client.test.ts +++ b/packages/sql/clickhouse/test/Client.test.ts @@ -1,7 +1,22 @@ import { ClickhouseClient } from "@effect/sql-clickhouse" import { assert, describe, it } from "@effect/vitest" import { Effect } from "effect" +import { TestClock } from "effect/testing" +import * as Reactivity from "effect/unstable/reactivity/Reactivity" import * as Statement from "effect/unstable/sql/Statement" +import { vi } from "vitest" + +let closeCalls = 0 + +vi.mock("@clickhouse/client", () => ({ + createClient: () => ({ + exec: () => new Promise(() => {}), + close: () => { + closeCalls++ + return Promise.resolve() + } + }) +})) describe("ClickhouseClient", () => { it("preserves fractional JavaScript numbers in inferred parameters", () => { @@ -10,4 +25,18 @@ describe("ClickhouseClient", () => { assert.strictEqual(query, "SELECT {p1: Float64}") }) + + it.effect("closes the client when the connection check times out", () => + Effect.gen(function*() { + closeCalls = 0 + const fiber = yield* Effect.forkDetach( + ClickhouseClient.make({ url: "http://localhost:8123" }).pipe(Effect.scoped) + ) + yield* Effect.yieldNow + yield* TestClock.adjust("5 seconds") + const result = fiber.pollUnsafe() + + assert.isDefined(result) + assert.strictEqual(closeCalls, 1) + }).pipe(Effect.provide(Reactivity.layer))) }) diff --git a/packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts b/packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts deleted file mode 100644 index ac7a4d99a7e..00000000000 --- a/packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { ClickhouseClient } from "@effect/sql-clickhouse" -import { assert, it } from "@effect/vitest" -import { Effect } from "effect" -import { TestClock } from "effect/testing" -import * as Reactivity from "effect/unstable/reactivity/Reactivity" -import { vi } from "vitest" - -let closeCalls = 0 - -vi.mock("@clickhouse/client", () => ({ - createClient: () => ({ - exec: () => new Promise(() => {}), - close: () => { - closeCalls++ - return Promise.resolve() - } - }) -})) - -it.effect("closes a ClickHouse client when its connection check times out", () => - Effect.gen(function*() { - closeCalls = 0 - const fiber = yield* Effect.forkDetach( - ClickhouseClient.make({ url: "http://localhost:8123" }).pipe(Effect.scoped) - ) - yield* Effect.yieldNow - yield* TestClock.adjust("5 seconds") - const result = fiber.pollUnsafe() - - assert.isDefined(result) - assert.strictEqual(closeCalls, 1) - }).pipe(Effect.provide(Reactivity.layer)))