fix: Format code and fix style issues #254
Conversation
…va into feat/docker
…va into feat/docker
WalkthroughRefactors streaming handling in AiChatV1ServiceImpl by extracting helper methods for line processing, error writing, and stream closing. Removes an empty line in a converter file and unused imports in ChatRequest. No public API or behavior changes; output format and control flow remain functionally the same. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as AiChatV1ServiceImpl
participant R as Remote AI API
participant O as OutputStream
C->>S: chatCompletion(request, response)
S->>R: Send chat request
R-->>S: Streaming HTTP response (body lines)
rect rgb(240,248,255)
note over S: Success path
S->>S: processLines(body, O)
loop For each line
S->>S: writeLine("data:" + line + "\n\n")
S->>O: write UTF-8 bytes, flush
end
end
alt Error occurs
S->>S: handleError(e, O)
S->>O: write error JSON event, flush
note over S,O: On IO failure, throw ServiceException CM326
end
S->>S: closeStream(O)
S-->>C: HTTP 200 with streamed body
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
base/src/main/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImpl.java (4)
95-107: Validate upstream is actually SSE (status + content-type) before processing lines.
Preemptively bail out if the upstream doesn’t return 2xx or text/event-stream to avoid misinterpreting non-SSE error payloads as data events.Apply this minimally invasive guard:
try { - HttpResponse<Stream<String>> response = httpClient.send( - requestBuilder.build(), HttpResponse.BodyHandlers.ofLines()); - processLines(response.body(), outputStream); + HttpResponse<Stream<String>> response = + httpClient.send(requestBuilder.build(), HttpResponse.BodyHandlers.ofLines()); + + boolean ok = response.statusCode() / 100 == 2; + boolean sse = response.headers() + .firstValue("Content-Type") + .map(v -> v.toLowerCase().startsWith("text/event-stream")) + .orElse(false); + if (!ok || !sse) { + handleError(new ServiceException(ExceptionEnum.CM326.getResultCode(), + "Upstream not SSE: status=" + response.statusCode()), outputStream); + return; + } + processLines(response.body(), outputStream);
109-113: Filter blank lines, not just empty strings.
SSE streams can contain whitespace-only keep-alives; use isBlank to avoid forwarding them.- try (Stream<String> filteredLines = lines.filter(line -> !line.isEmpty())) { + try (Stream<String> filteredLines = lines.filter(line -> !line.isBlank())) { filteredLines.forEach(line -> writeLine(line, outputStream)); }
115-129: Preserve non-data SSE fields (event/id/retry) instead of rewriting them as data.
Prefixing every non-data line with "data:" can corrupt valid SSE fields from providers. Pass through known SSE fields unchanged; only add "data:" when the line has no field.- if (!line.startsWith("data:")) { + if (!(line.startsWith("data:") || line.startsWith("event:") + || line.startsWith("id:") || line.startsWith("retry:"))) { line = "data: " + line; }Optional: to reduce syscall overhead, consider batching flushes (e.g., flush every N events) if end-to-end latency budget allows.
131-139: Emit explicit SSE error event and guard null messages.
Includeevent: errorfor consumers and handle null exception messages.- try { - String errorEvent = "data: " + JsonUtils.encode(Map.of("error", e.getMessage())) + "\n\n"; + try { + String message = (e.getMessage() != null) ? e.getMessage() : e.toString(); + String errorEvent = "event: error\n" + + "data: " + JsonUtils.encode(Map.of("error", message)) + "\n\n"; outputStream.write(errorEvent.getBytes(StandardCharsets.UTF_8)); outputStream.flush(); } catch (IOException ioException) { throw new ServiceException(ExceptionEnum.CM326.getResultCode(), ExceptionEnum.CM326.getResultMsg()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
base/src/main/java/com/tinyengine/it/common/converter/StreamingResponseBodyConverter.java(0 hunks)base/src/main/java/com/tinyengine/it/model/dto/ChatRequest.java(0 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImpl.java(2 hunks)
💤 Files with no reviewable changes (2)
- base/src/main/java/com/tinyengine/it/model/dto/ChatRequest.java
- base/src/main/java/com/tinyengine/it/common/converter/StreamingResponseBodyConverter.java
🧰 Additional context used
🧬 Code graph analysis (1)
base/src/main/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImpl.java (1)
base/src/main/java/com/tinyengine/it/common/utils/JsonUtils.java (1)
JsonUtils(45-349)
🔇 Additional comments (2)
base/src/main/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImpl.java (2)
28-28: LGTM: Needed import for new helpers.
Importing OutputStream is appropriate for the refactored streaming helpers.
141-147: LGTM: Close quietly in finally.
Quiet close is appropriate here to avoid masking the original error.
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit