Make ClickHouse startup timeouts interruptible and close the client by fubhy · Pull Request #6907 · Effect-TS/effect

fubhy · 2026-08-03T05:59:58Z

Summary

A pending startup connectivity check outlives the advertised five-second timeout and leaves the newly created ClickHouse client without a close finalizer.

Important

This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.

Connection timeout cannot interrupt startup acquisition

Module: clickhouse/ClickhouseClient
Audit ID: sql-adapters-ch-1
Severity / confidence: high / high

What happens

A pending startup connectivity check outlives the advertised five-second timeout and leaves the newly created ClickHouse client without a close finalizer.

Why it happens

The pending client.exec is the acquisition of Effect.acquireRelease, so acquisition is uninterruptible and the client.close release is not installed until the check succeeds.

Expected behavior

The scoped constructor must enforce its five-second connectivity timeout and close the SDK client when startup does not complete.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

packages/sql/clickhouse/src/ClickhouseClient.ts:178-201

View problematic code at packages/sql/clickhouse/src/ClickhouseClient.ts:178-201

    const client = Clickhouse.createClient(options)

    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(
      Effect.timeoutOrElse({
        duration: Duration.seconds(5),
        orElse: () =>
          Effect.fail(
            new SqlError({
              reason: new ConnectionError({
                message: "ClickhouseClient: Connection timeout",
                cause: new Error("connection timeout"),
                operation: "connect"
              })
            })
          )
      })
    )

View exact lines on GitHub

Reproduction

pnpm test --run packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts

Observed failure: FAIL: the make fiber remained pending after five virtual seconds.

Implementation handoff

The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.

Start with the pinned implementation excerpts and the Why it happens analysis above.
Change the implementation so it satisfies the stated Expected behavior; do not weaken or remove the reproduction assertions.
Run the focused reproduction command(s) and confirm the observed failures become passing tests:

pnpm test --run packages/sql/clickhouse/test/ConnectTimeoutLeakRepro.test.ts

Run the affected package's existing tests, then the repository lint and type checks before requesting review.

Audit provenance

Audit base: c9b56ab507f224426ee8388dc450da447ec4715f
Reproduction base: c9b56ab507f224426ee8388dc450da447ec4715f
Findings: sql-adapters-ch-1
Initial patch: focused reproduction tests; implementation fix pending

Closes EFF-343

changeset-bot · 2026-08-03T06:00:02Z

🦋 Changeset detected

Latest commit: 91f1135

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
@effect/sql-clickhouse	Patch
effect	Patch
@effect/opentelemetry	Patch
@effect/platform-browser	Patch
@effect/platform-bun	Patch
@effect/platform-deno	Patch
@effect/platform-node-shared	Patch
@effect/platform-node	Patch
@effect/vitest	Patch
@effect/ai-anthropic	Patch
@effect/ai-openai-compat	Patch
@effect/ai-openai	Patch
@effect/ai-openrouter	Patch
@effect/atom-react	Patch
@effect/atom-solid	Patch
@effect/atom-vue	Patch
@effect/sql-d1	Patch
@effect/sql-libsql	Patch
@effect/sql-mssql	Patch
@effect/sql-mysql2	Patch
@effect/sql-pg	Patch
@effect/sql-pglite	Patch
@effect/sql-sqlite-bun	Patch
@effect/sql-sqlite-do	Patch
@effect/sql-sqlite-node	Patch
@effect/sql-sqlite-react-native	Patch
@effect/sql-sqlite-wasm	Patch
@effect/docgen	Patch
@effect/doctest	Patch
@effect/openapi-generator	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pullfrog

✅ No new issues found.

Reviewed changes — a single reproduction test file for the ClickHouse connect-timeout resource leak audit issue.

ConnectTimeoutLeakRepro.test.ts: mocks @clickhouse/client with a never-resolving exec promise, runs ClickhouseClient.make in a detached fiber, advances TestClock by 5 seconds, and asserts close() was called exactly once. Expected to fail on current main — correctly reproduces the bug.

The test follows established patterns: TestClock for time simulation (forkDetach → yieldNow → TestClock.adjust → pollUnsafe), vi.mock for driver isolation (matching the existing SqlErrorClassification.test.ts pattern), and assert from @effect/vitest. The assertion on closeCalls === 1 is exact and verifiable — it will only pass once the implementation ensures the underlying SDK client is closed on timeout.

^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏}

pullfrog

✅ No new issues found.

Reviewed changes — the implementation fix moving acquireRelease to wrap createClient() so the close() finalizer is registered before the startup connectivity check, plus test consolidation.

Fixed ClickhouseClient.ts: moved Effect.acquireRelease to wrap Clickhouse.createClient() on line 178, registering the close() finalizer immediately. The SELECT 1 connectivity check on line 183 is now a regular yield* with the 5-second timeoutOrElse — interruptible, and the scope ensures close() runs regardless of whether the check succeeds or times out.
Consolidated test from ConnectTimeoutLeakRepro.test.ts into Client.test.ts: moved the mock + detached-fiber timeout assertion into the main test file, deleted the standalone repro file.
Added changeset: patch-level "@effect/sql-clickhouse" entry.

^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏}

pullfrog

✅ No new issues found.

Reviewed changes — the implementation fix was rebased onto the current main; the code is unchanged from the previously approved version.

Rebased ClickhouseClient.ts: Effect.acquireRelease wraps Clickhouse.createClient() on line 178, registering the close() finalizer before the SELECT 1 connectivity check. The check on line 183 is now a regular yield* with the 5-second timeoutOrElse — interruptible, and the scope ensures close() runs regardless of outcome.
Test consolidated into Client.test.ts: the standalone ConnectTimeoutLeakRepro.test.ts was deleted; its test now lives alongside the existing fractional-number test. Uses vi.mock for driver isolation, forkDetach → TestClock.adjust → pollUnsafe for timeout simulation, and Reactivity.layer for the test environment.
Changeset: patch-level "@effect/sql-clickhouse" entry added.

^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏}

github-actions · 2026-08-03T21:52:53Z

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name	Current Size	Previous Size	Difference
`basic.ts`	7.06 KB	7.06 KB	0.00 KB (0.00%)
`batching.ts`	9.86 KB	9.86 KB	0.00 KB (0.00%)
`brand.ts`	6.34 KB	6.34 KB	0.00 KB (0.00%)
`cache.ts`	10.62 KB	10.62 KB	0.00 KB (0.00%)
`config.ts`	20.60 KB	20.60 KB	0.00 KB (0.00%)
`differ.ts`	20.20 KB	20.20 KB	0.00 KB (0.00%)
`http-client.ts`	21.49 KB	21.49 KB	0.00 KB (0.00%)
`logger.ts`	10.76 KB	10.76 KB	0.00 KB (0.00%)
`metric.ts`	8.99 KB	8.99 KB	0.00 KB (0.00%)
`optic.ts`	7.18 KB	7.18 KB	0.00 KB (0.00%)
`pubsub.ts`	14.90 KB	14.90 KB	0.00 KB (0.00%)
`queue.ts`	11.58 KB	11.58 KB	0.00 KB (0.00%)
`schedule.ts`	10.74 KB	10.74 KB	0.00 KB (0.00%)
`schema-class.ts`	19.14 KB	19.14 KB	0.00 KB (0.00%)
`schema-fromJsonSchemaDocument.ts`	28.96 KB	28.96 KB	0.00 KB (0.00%)
`schema-representation-roundtrip.ts`	25.29 KB	25.29 KB	0.00 KB (0.00%)
`schema-string-transformation.ts`	13.30 KB	13.30 KB	0.00 KB (0.00%)
`schema-string.ts`	10.94 KB	10.94 KB	0.00 KB (0.00%)
`schema-template-literal.ts`	15.17 KB	15.17 KB	0.00 KB (0.00%)
`schema-toArbitraryLazy.ts`	21.94 KB	21.94 KB	0.00 KB (0.00%)
`schema-toCodeDocument.ts`	24.34 KB	24.34 KB	0.00 KB (0.00%)
`schema-toCodecJson.ts`	19.18 KB	19.18 KB	0.00 KB (0.00%)
`schema-toEquivalence.ts`	19.01 KB	19.01 KB	0.00 KB (0.00%)
`schema-toFormatter.ts`	18.87 KB	18.87 KB	0.00 KB (0.00%)
`schema-toJsonSchemaDocument.ts`	22.60 KB	22.60 KB	0.00 KB (0.00%)
`schema-toRepresentation.ts`	19.52 KB	19.52 KB	0.00 KB (0.00%)
`schema.ts`	18.41 KB	18.41 KB	0.00 KB (0.00%)
`stm.ts`	12.54 KB	12.54 KB	0.00 KB (0.00%)
`stream.ts`	9.79 KB	9.79 KB	0.00 KB (0.00%)

github-project-automation Bot added this to PR Backlog Aug 3, 2026

github-project-automation Bot moved this to Discussion Ongoing in PR Backlog Aug 3, 2026

effect-slopcop Bot added the 4.0 label Aug 3, 2026

pullfrog Bot approved these changes Aug 3, 2026

View reviewed changes

fubhy changed the title ~~Add reproduction for clickhouse/ClickhouseClient issue~~ Make ClickHouse startup timeouts interruptible and close the client Aug 3, 2026

effect-slopcop Bot added the bug Something isn't working label Aug 3, 2026

fubhy added the audit Findings originating from the Effect runtime correctness audit label Aug 3, 2026

Add reproduction for clickhouse/ClickhouseClient issue

80fec2c

pullfrog Bot approved these changes Aug 3, 2026

View reviewed changes

Fix ClickHouse connect timeout cleanup

91f1135

tim-smart force-pushed the audit/repro-sql-clickhouse-connect-timeout-leak branch from 514409c to 91f1135 Compare August 3, 2026 21:28

tim-smart enabled auto-merge (squash) August 3, 2026 21:32

pullfrog Bot approved these changes Aug 3, 2026

View reviewed changes

tim-smart merged commit c8fd57e into main Aug 3, 2026
17 checks passed

tim-smart deleted the audit/repro-sql-clickhouse-connect-timeout-leak branch August 3, 2026 21:48

github-project-automation Bot moved this from Discussion Ongoing to Done in PR Backlog Aug 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Make ClickHouse startup timeouts interruptible and close the client - #6907

Make ClickHouse startup timeouts interruptible and close the client#6907
tim-smart merged 2 commits into
mainfrom
audit/repro-sql-clickhouse-connect-timeout-leak

fubhy commented Aug 3, 2026 •

edited by tim-smart

Loading

Uh oh!

changeset-bot Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

pullfrog Bot left a comment

Uh oh!

pullfrog Bot left a comment

Uh oh!

pullfrog Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

fubhy commented Aug 3, 2026 • edited by tim-smart Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Connection timeout cannot interrupt startup acquisition

What happens

Why it happens

Expected behavior

Relevant implementation

Reproduction

Implementation handoff

Audit provenance

Uh oh!

changeset-bot Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 3, 2026

Bundle Size Analysis

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fubhy commented Aug 3, 2026 •

edited by tim-smart

Loading

changeset-bot Bot commented Aug 3, 2026 •

edited

Loading