feat: Add support for enhanced response logging via HttpFormatter
#390
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
📝 WalkthroughSummary by CodeRabbit
WalkthroughPropagates a new HttpFormatter through stub-building and response layers, adds formatted verbose logging for responses and streamed chunks, makes AbstractResponseDefinition.contentType non‑nullable and adds headerList/delay/responseBody constructor properties, introduces SSE dependency and encoding changes, and updates tests and call sites. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Server as MokksyServer
participant Step as BuildingStep
participant Builder as ResponseDefinitionBuilder / StreamingResponseDefinitionBuilder
participant Resp as ResponseDefinition / StreamResponseDefinition
participant Call as Ktor ApplicationCall
participant Fmt as HttpFormatter
Dev->>Server: register stub / define route
Server->>Step: create BuildingStep(..., formatter = httpFormatter)
Step->>Builder: create builder(..., formatter)
Builder->>Resp: build(..., formatter)
Call->>Resp: writeResponse(call, verbose, chunkContentType?)
alt verbose
Resp->>Fmt: formatResponse(...) / formatResponseChunk(...)
Fmt-->>Resp: formatted text
Resp->>Call: send headers/body/chunks (and log formatted output)
else non-verbose
Resp->>Call: send headers/body/chunks (no formatted log)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinition.kt (1)
55-75: Honor runtime body overrides inResponseDefinition.writeResponse.
AbstractResponseDefinition.withResponseBodynow allows call sites to mutate the payload by settingresponseBody, but this override is never read here—we still log and sendbody. As a result, any dynamic response shaping is silently ignored. Please resolve the effective payload once (e.g.,val effectiveBody = responseBody ?: body) and use it for both logging andcall.respond.- if (verbose) { + val effectiveBody = responseBody ?: body + if (verbose) { call.application.log.debug( "Sending:\n---\n${ formatter.formatResponse( httpVersion = call.request.httpVersion, headers = call.response.headers, contentType = this.contentType, status = httpStatus, - body = body?.toString(), + body = effectiveBody?.toString(), ) }---\n", ) } try { - call.respond( + val payload: Any = effectiveBody ?: "" + call.respond( status = httpStatus, - message = body ?: "" as Any, + message = payload, )mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/AbstractResponseDefinition.kt (1)
32-32: Avoid exposing mutableresponseBodyas public.Make it
protectedto restrict mutation to subclasses. External mutation should go throughwithResponseBody.Apply this diff:
- public var responseBody: T? = null, + protected var responseBody: T? = null,mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinitionBuilders.kt (1)
53-55: KeephttpStatusCodeandhttpStatusconsistent.
httpStatus(status: Int)updates onlyhttpStatus, leavinghttpStatusCodestale.Apply this diff:
- public fun httpStatus(status: Int) { - this.httpStatus = HttpStatusCode.fromValue(status) - } + public fun httpStatus(status: Int) { + this.httpStatusCode = status + this.httpStatus = HttpStatusCode.fromValue(status) + }mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/SseStreamResponseDefinition.kt (2)
41-44: Invalid logger usage: Ktor Logger doesn’t support SLF4J-style{}placeholders.This won’t compile;
debugaccepts a singleString.Apply this diff:
- if (verbose) { - call.application.log.debug("Sending {}: {}", httpStatus, it) - } + if (verbose) { + call.application.log.debug("Sending $httpStatus: $it") + }
64-66: Honor configured status for SSE responses.Use
httpStatusinstead of hardcodedHttpStatusCode.OK.Apply this diff:
- call.response.status(HttpStatusCode.OK) + call.response.status(httpStatus)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/StreamResponseDefinition.kt (3)
170-199: Apply headers before streaming from Flow.
headersLambdaandheaderListare not applied; responses may miss configured headers.Apply this diff:
override suspend fun writeResponse( call: ApplicationCall, verbose: Boolean, chunkContentType: ContentType?, ) { - when { + // Apply configured headers + headers?.invoke(call.response.headers) + for ((name, value) in headerList) { + call.response.headers.append(name, value) + } + when { chunkFlow != null -> { call.response.cacheControl(CacheControl.NoCache(null)) call.respondBytesWriter( status = this.httpStatus, contentType = this.contentType, ) {
202-215: Parity: log formatted headers for list-based chunks too.Add the same verbose header log as in the Flow branch.
Apply this diff:
call.respondBytesWriter( status = this.httpStatus, contentType = this.contentType, ) { + if (verbose) { + call.application.log.debug( + "Sending:\n---\n${ + formatter.formatResponseHeader( + httpVersion = call.request.httpVersion, + headers = call.response.headers, + status = httpStatus, + ) + }", + ) + } writeChunksFromList( writer = this, verbose = verbose, logger = call.application.log, chunkContentType = chunkContentType, )
137-138: Remove unused overload ofwriteChunksFromFlow(session: ServerSSESession)
This method (and its internal@Suppress("unused")annotation) to eliminate dead code and avoid
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/BuildingStep.kt(5 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/MokksyServer.kt(1 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/AbstractResponseDefinition.kt(2 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinition.kt(3 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinitionBuilders.kt(5 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/SseStreamResponseDefinition.kt(2 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/StreamResponseDefinition.kt(8 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/utils/logger/HttpFormatter.kt(5 hunks)mokksy/src/commonTest/kotlin/me/kpavlov/mokksy/BuildingStepTest.kt(2 hunks)mokksy/src/jvmMain/kotlin/me/kpavlov/mokksy/utils/Base64Urls.jvm.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/utils/logger/HttpFormatter.kt (2)
ai-mocks-a2a/src/jvmTest/kotlin/me/kpavlov/mokksy/utils/logger/HighlightingTest.kt (1)
colorize(73-76)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/utils/logger/Highlighting.kt (1)
highlightBody(15-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
🔇 Additional comments (8)
mokksy/src/jvmMain/kotlin/me/kpavlov/mokksy/utils/Base64Urls.jvm.kt (1)
11-13: Signature reflow keeps API intactThe multiline signature with trailing comma is idiomatic Kotlin and preserves behavior. Looks good.
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/BuildingStep.kt (1)
70-108: Formatter plumbing is consistentForwarding the injected
HttpFormatterinto both the regular and streaming response builders ensures all response paths share the same formatting logic. Nice work.mokksy/src/commonTest/kotlin/me/kpavlov/mokksy/BuildingStepTest.kt (1)
49-56: Test setup updated correctlyPassing a concrete
HttpFormatter()into the subject mirrors the production wiring and keeps the fixture aligned with the new constructor signature. 👍mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/MokksyServer.kt (1)
195-201: Formatter propagates from server entry pointInjecting the shared
httpFormatterinto eachBuildingStepmaintains consistent formatting across all stub builders. Looks good.mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinitionBuilders.kt (1)
87-88: Formatter wiring LGTM.Passing
HttpFormatterthrough the builders to concrete definitions is correct.mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/SseStreamResponseDefinition.kt (1)
24-26: Formatter injection LGTM.Constructor change correctly forwards
formatterto the superclass.mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/StreamResponseDefinition.kt (1)
60-61: Formatter injection LGTM.Passing
formatterinto the streaming response definition is correct.mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/AbstractResponseDefinition.kt (1)
37-38: All writeResponse overrides updated withchunkContentType.
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/AbstractResponseDefinition.kt
Show resolved
Hide resolved
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinitionBuilders.kt
Outdated
Show resolved
Hide resolved
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/SseStreamResponseDefinition.kt
Show resolved
Hide resolved
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/StreamResponseDefinition.kt
Show resolved
Hide resolved
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/utils/logger/HttpFormatter.kt
Show resolved
Hide resolved
kpavlov
force-pushed
the
colorize-output
branch
2 times, most recently
from
457a003 to
cdd06e2
Compare
- Enhanced response lifecycle methods across multiple classes to support verbose logging with chunk details. - Updated `ResponseDefinition` and its builders to integrate `HttpFormatter` for better structured output.
kpavlov
force-pushed
the
colorize-output
branch
from
cdd06e2 to
ab97da3
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/AbstractResponseDefinition.kt (1)
18-31: Document the newdelayandresponseBodyproperties.The constructor now exposes
delayandresponseBody, but the KDoc still omits them. Updating the doc keeps the contract clear for consumers.Apply this diff to extend the property list:
* @property headers A lambda function for configuring the response headers. Defaults to `null`. * @property headerList A list of header key-value pairs to populate the response headers. Defaults to an empty list. + * @property delay A delay applied before sending the response. Defaults to [Duration.ZERO]. + * @property responseBody The optional response payload associated with this definition. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
ai-mocks-gemini/build.gradle.kts(1 hunks)ai-mocks-gemini/src/commonMain/kotlin/me/kpavlov/aimocks/gemini/content/GeminiStreamingContentBuildingStep.kt(2 hunks)gradle/libs.versions.toml(1 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/BuildingStep.kt(5 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/MokksyServer.kt(1 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/AbstractResponseDefinition.kt(1 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinition.kt(3 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinitionBuilders.kt(6 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/SseStreamResponseDefinition.kt(2 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/StreamResponseDefinition.kt(8 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/utils/logger/HttpFormatter.kt(5 hunks)mokksy/src/commonTest/kotlin/me/kpavlov/mokksy/BuildingStepTest.kt(2 hunks)mokksy/src/jvmMain/kotlin/me/kpavlov/mokksy/utils/Base64Urls.jvm.kt(1 hunks)
👮 Files not reviewed due to content moderation or server errors (6)
- ai-mocks-gemini/build.gradle.kts
- gradle/libs.versions.toml
- mokksy/src/commonTest/kotlin/me/kpavlov/mokksy/BuildingStepTest.kt
- mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/MokksyServer.kt
- mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinition.kt
- mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/BuildingStep.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
mokksy/src/jvmMain/kotlin/me/kpavlov/mokksy/utils/Base64Urls.jvm.kt (1)
11-13: Formatting-only change looks goodExpanding the parameter list for readability keeps behavior intact. No concerns.
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/utils/logger/HttpFormatter.kt (1)
189-193: Compile error: iterateallValues()without callingentries()Line 190 currently invokes
headers.allValues().entries()butallValues()already returns aMap<String, List<String>>. Callingentries()on that map won’t compile. Iterate the map directly so the formatter can build header lines.Apply this diff:
- headers.allValues().entries().forEach { (key, value) -> - appendLine(header(key, value)) - } + headers + .allValues() + .forEach { (key, values) -> + appendLine(header(key, values)) + }mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/ResponseDefinitionBuilders.kt (1)
9-9: Remove JVM-onlyCollectionsusage incommonMain.
java.util.Collectionsisn’t available tocommonMain, so this won’t compile for non-JVM targets. Copy the headers withtoList()(or another multiplatform-safe option) and drop the import.-import me.kpavlov.mokksy.utils.logger.HttpFormatter -import java.util.Collections +import me.kpavlov.mokksy.utils.logger.HttpFormatter @@ - headerList = Collections.unmodifiableList(headers), + headerList = headers.toList(),Also applies to: 149-154
ai-mocks-gemini/src/commonMain/kotlin/me/kpavlov/aimocks/gemini/content/GeminiStreamingContentBuildingStep.kt (1)
95-100: TypedServerSentEvent serialization looks solid.Line 96: Switching to
TypedServerSentEventand delegating the payload encoding toJson.encodeToStringremoves the brittle, hand-crafted SSE formatting and yields well-formed events. Nicely done.
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/SseStreamResponseDefinition.kt
Outdated
Show resolved
Hide resolved
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/response/StreamResponseDefinition.kt
Show resolved
Hide resolved
| ktor-server-double-receive = { module = "io.ktor:ktor-server-double-receive" } | ||
| ktor-server-netty = { module = "io.ktor:ktor-server-netty" } | ||
| ktor-server-sse = { module = "io.ktor:ktor-server-sse" } | ||
| ktor-sse = { module = "io.ktor:ktor-sse" } |
There was a problem hiding this comment.
io.ktor:ktor-sse artifact doesn’t exist in Ktor 3.3.0
Ktor only publishes ktor-server-sse / ktor-client-sse. This alias will break dependency resolution as Maven can’t find io.ktor:ktor-sse. Please point the alias to the actual module you need (e.g., ktor-client-sse) or drop it.
🤖 Prompt for AI Agents
In gradle/libs.versions.toml around line 67, the alias ktor-sse is pointing to a
non-existent artifact "io.ktor:ktor-sse"; replace it with the correct module
published by Ktor 3.3.0 (for example "io.ktor:ktor-client-sse" or
"io.ktor:ktor-server-sse" depending on whether you need client or server SSE) or
remove the alias entirely so dependency resolution won't fail.
- Update `ResponseDefinition` to utilize `effectiveBody` for improved logging and response payload handling. - Allow application of custom headers in `StreamResponseDefinition`. - Fix logging format in `SseStreamResponseDefinition` and ensure proper usage of `httpStatus`. - Add support for configuring `httpStatusCode` in response builders.
Improve response logging via
HttpFormatter