KTOR-646 Log client-disconnect IOException at TRACE in Netty HTTP/1 handler

[fru1tworld]

Subsystem
Server, Netty

Motivation
KTOR-646 / #1030 — NettyHttp1Handler.exceptionCaught logs every IOException at DEBUG, so routine client disconnects ("Connection reset by peer") show up as noise in default DEBUG logs.

Solution
Lower the log level to TRACE. Information stays available for deep debugging without polluting DEBUG output.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4522b1f2-ced4-42ec-857f-2a3983a1792b

📥 Commits

Reviewing files that changed from the base of the PR and between 010b9f6 and d616231.

📒 Files selected for processing (1)

• ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt

---

📝 Walkthrough

Walkthrough

IOException handling in the Netty HTTP/1 handler now logs at TRACE (not DEBUG). A new unit test asserts a single TRACE-level "I/O operation failed" log is emitted for a client-disconnect IOException using an EmbeddedChannel and Logback ListAppender.

Changes

Cohort / File(s)	Summary
Production Code ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/http1/NettyHttp1Handler.kt	Adjusted exceptionCaught to log IOException at TRACE instead of DEBUG; behavior still cancels handlerJob and closes the channel.
Test Coverage ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt	Added Netty-specific regression test that configures a TRACE logger and ListAppender, triggers exceptionCaught with a client-disconnect IOException via EmbeddedChannel, and asserts a single TRACE "I/O operation failed" log entry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: lowering the log level for IOException in Netty HTTP/1 handler from DEBUG to TRACE, and is specific to the KTOR-646 issue.
Description check	✅ Passed	The description follows the template with all required sections: Subsystem, Motivation, and Solution are complete and provide clear context for the change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

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

[coderabbitai]

🧹 Nitpick comments (1)

ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt (1)

210-226: Optional: remove redundant appender detachment.

logger.detachAppender(listAppender) runs both before assertions (Line 210) and again in finally (Line 225). Keeping it only in finally simplifies cleanup without changing behavior.

♻️ Suggested simplification

         try {
             channel.pipeline().fireExceptionCaught(IOException("Connection reset by peer"))
-
-            logger.detachAppender(listAppender)
 
             val ioOpFailedEvents = listAppender.list.filter { it.formattedMessage == "I/O operation failed" }
             assertEquals(
                 1,
                 ioOpFailedEvents.size,
@@
         } finally {
             logger.detachAppender(listAppender)
             logger.level = previousLevel
             channel.finishAndReleaseAll()
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt`
around lines 210 - 226, The test currently calls
logger.detachAppender(listAppender) twice (once before the assertions and again
in the finally block); remove the earlier redundant call and keep only the
cleanup in the finally block so the appender is always detached and cleanup is
centralized—update the block around the ioOpFailedEvents assertions to stop
calling logger.detachAppender(listAppender) before assertions and rely on the
existing finally that restores logger.level and detaches the appender.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt`:
- Around line 210-226: The test currently calls
logger.detachAppender(listAppender) twice (once before the assertions and again
in the finally block); remove the earlier redundant call and keep only the
cleanup in the finally block so the appender is always detached and cleanup is
centralized—update the block around the ioOpFailedEvents assertions to stop
calling logger.detachAppender(listAppender) before assertions and rely on the
existing finally that restores logger.level and detaches the appender.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 548cf7fe-8a06-4e1d-9f6f-89aba19033ae

📥 Commits

Reviewing files that changed from the base of the PR and between 36c42ea and 010b9f6.

📒 Files selected for processing (2)

• ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/http1/NettyHttp1Handler.kt
• ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt

[bjhham]

Thanks for the fix. One small comment wrt naming

[bjhham]

Best to simplify the test name here to describe the high-level behavior like client disconnect is logged and leave the level as an implementation detail. We generally avoid referencing YouTrack issue numbers in fixes too.

[fru1tworld]

Thanks for the review! Addressed both points in d616231:

• Renamed the test to client disconnect is logged at trace level and dropped the YouTrack number from both the test name and the logger name.
• Removed the redundant detachAppender call before assertions (kept the one in finally).

[bjhham]

Looks good, thanks!