KTOR-6761 Add Apache5EngineConfig.configureConnectionManager to simplify configuration

[marychatte]

Subsystem
Client, Apache5

Motivation
KTOR-6761 Apache5: Simplify configuration

Solution
Initially, the configuration looked like:

HttpClient(Apache) {
    engine {
        customizeClient {
            setMaxConnTotal(10_000)
            setMaxConnPerRoute(1_000)
        }
    }
}

But after upgrading to Apache5, the configuration started to look like:

HttpClient(Apache5) {
    engine {
        customizeClient {
            setConnectionManager(
                PoolingAsyncClientConnectionManagerBuilder.create()
                    .setMaxConnTotal(10_000)
                    .setMaxConnPerRoute(1_000)
                    .build()
            )
        }
    }
}

Because users need to create and set ConnectionManager now, it overrides the Ktor-managed connection manager here. For example, if the HttpTimeout plugin is used, its settings will be ignored.

The idea is to provide a configureConnectionManager function, so users won't override ConnectionManager but still can configure it.

[coderabbitai]

Walkthrough

Adds a new DSL hook configureConnectionManager to Apache5EngineConfig and applies it when building the Apache5 connection manager; introduces an SSE /sse/delay/{time} test route and two Apache5 engine tests validating socket timeout behavior and precedence between engine-level connection config and HttpTimeout.

Changes

Cohort / File(s)	Summary
Apache5 engine wiring ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt	After building the default ConnectionConfig, applies config.configureConnectionManager to the PoolingAsyncClientConnectionManagerBuilder via apply, enabling user-supplied customization of the connection manager.
Apache5 engine config API ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.kt	Adds an internal configureConnectionManager: PoolingAsyncClientConnectionManagerBuilder.() -> Unit and a public DSL method configureConnectionManager(block: PoolingAsyncClientConnectionManagerBuilder.() -> Unit) to allow configuring the connection manager.
Apache5 engine tests ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt	Adds two tests: testSocketTimeoutWithCustomConnectionManager (verifies socket timeout leads to ClosedByteChannelException with SocketTimeoutException root cause) and testCustomTimeoutOverridesHttpTimeout (verifies engine-level ConnectionConfig socket timeout can override HttpTimeout).
Test server SSE route ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt	Adds a new route GET /sse/delay/{time} that delays by the given milliseconds then writes a single SSE event ("data: hello\n").
Public API descriptor ktor-client/ktor-client-apache5/api/ktor-client-apache5.api	Declares the new public function configureConnectionManager on Apache5EngineConfig (Kotlin Function1 ABI entry).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• bjhham

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely states the primary change — adding Apache5EngineConfig.configureConnectionManager — and includes the issue ID (KTOR-6761) for traceability; it directly reflects the new public API and the intent to simplify configuration, so a teammate scanning PR history will understand the main change.
Description Check	✅ Passed	The PR description follows the repository template by including Subsystem, Motivation (with the KTOR-6761 link), and a Solution that explains the problem and shows before-and-after code examples, giving reviewers sufficient context and rationale for the change.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch marychatte/KTOR-6761-Apache5-Simplify-configuration-Connection-Manager

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[bjhham]

Looks like it will need an ABI dump, and you can point it to release/3.x if you want to include it in 3.3.1 but otherwise all good nice to see some older ones go 👍

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt (1)

77-91: Clamp negative delay to avoid IllegalArgumentException in delay()

If a client passes a negative {time}, delay(time) will throw. Clamp to non‑negative.

Apply this diff:

-                get("delay/{time}") {
-                    val time = call.parameters["time"]?.toLongOrNull() ?: 0L
+                get("delay/{time}") {
+                    val time = (call.parameters["time"]?.toLongOrNull() ?: 0L).coerceAtLeast(0L)

ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (2)

28-39: Close clients in tests to avoid leaked threads/resources

Wrap client usage in use { ... } or call client.close() in a finally block.

-        val client = HttpClient(Apache5) {
+        val client = HttpClient(Apache5) {
             engine {
                 configureConnectionManager = {
                     setMaxConnPerRoute(1_000)
                     setMaxConnTotal(2_000)
                 }
             }
             install(HttpTimeout) {
                 socketTimeoutMillis = 500
             }
-        }
+        }.also { /* consider client.use { ... } pattern below */ }

Or:

HttpClient(Apache5) { /* config */ }.use { client ->
    // test body
}

---

50-51: Remove redundant Unit

The last Unit is unnecessary in a runBlocking test.

-        Unit

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a221cbc and 31ca0fe.

📒 Files selected for processing (4)

• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt (1 hunks)
• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.kt (2 hunks)
• ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (1 hunks)
• ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{kt,kts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files

Files:

• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt
• ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt
• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.kt
• ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt

**/*.kt

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions

Files:

• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt
• ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt
• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.kt
• ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt

🧠 Learnings (1)

📚 Learning: 2025-09-05T12:46:14.074Z

Learnt from: bjhham
PR: ktorio/ktor#4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:90-100
Timestamp: 2025-09-05T12:46:14.074Z
Learning: The ByteReadChannel.readAvailable(ByteBuffer) method in Ktor IO automatically calls awaitContent() internally when the read buffer is exhausted. When it returns 0, it has already suspended and waited for data to become available, so adding explicit awaitContent() calls is redundant and incorrect.

Applied to files:

• ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt

🧬 Code graph analysis (2)

ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt (1)

ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpRequestRetry.kt (1)

• delay (243-245)

ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (1)

ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt (1)

• engine (57-121)

🔇 Additional comments (1)

ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (1)

56-69: This usage will keep working after the API tweak

Your lambda returns the builder, but assigning it to a Unit configurator remains source‑compatible; the return value is simply ignored.

Please re-run the Apache5 tests after applying the API change to confirm no source breakages.

[osipxd]

and you can point it to release/3.x if you want to include it in 3.3.1

Isn't it prohibited to add new APIs in patch versions? While this change is forward-compatible it breaks backward compatibility.

[marychatte]

I have a similar question 😅 I want to include it in 3.4.0

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

ktor-client/ktor-client-apache5/api/ktor-client-apache5.api (1)

11-11: Optional: avoid exposing Apache builder type directly to future‑proof ABI.

The Kotlin signature is a Function1 with a receiver of PoolingAsyncClientConnectionManagerBuilder. That type is baked into Kotlin metadata; if the underlying Apache builder type changes in the future, client code compiled against today’s metadata can face runtime ClassCastException. Consider introducing a Ktor-owned scope (thin wrapper) and accepting that instead, delegating to the Apache builder under the hood.

Example sketch:

// new public API surface you control
interface Apache5ConnectionManagerBuilderScope {
    val delegate: org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder
    // optionally expose only the knobs you want stable
}

// public API
fun configureConnectionManager(block: Apache5ConnectionManagerBuilderScope.() -> Unit)

// internal impl adapts the scope to the real builder

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31ca0fe and fe4e4a8.

📒 Files selected for processing (4)

• ktor-client/ktor-client-apache5/api/ktor-client-apache5.api (1 hunks)
• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt (1 hunks)
• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.kt (3 hunks)
• ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.kt
• ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt
• ktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.kt

🧰 Additional context used

📓 Path-based instructions (1)

**/api/**

📄 CodeRabbit inference engine (CLAUDE.md)

Track all public API changes in the /api/ directories

Files:

• ktor-client/ktor-client-apache5/api/ktor-client-apache5.api