-
Notifications
You must be signed in to change notification settings - Fork 90
fix: When chat event type is conversation.chat.completed, a NPE will … #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…be thrown; Error occurred: Cannot invoke "com.coze.openapi.client.connversations.message.model.Message.getType()" because the return value of "com.coze.openapi.client.chat.model.ChatEvent.getMessage()" is null
|
|
WalkthroughAdds a null-check before accessing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
example/src/main/java/example/chat/StreamChatExample.java (2)
38-38: Remove the extra semicolon.There's an unnecessary semicolon after the
build()statement.- .build(); - ; + .build();
78-83: Consider translating comments to English for consistency.The comments on lines 78-80 are in Chinese, while all other comments in the file are in English. For codebase consistency and broader team accessibility, consider translating them.
- // 为了防止程序立即退出,添加一个简单的等待 + // Add a simple wait to prevent the program from exiting immediately try { - Thread.sleep(5000); // 等待5秒钟 + Thread.sleep(5000); // Wait for 5 seconds } catch (InterruptedException e) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example/src/main/java/example/chat/StreamChatExample.java(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
example/src/main/java/example/chat/StreamChatExample.java
[error] 58-58: spotless check failed. The file has formatting violations detected by the Spotless Maven Plugin. Run 'mvn spotless:apply' to fix code style issues in this file.
🔇 Additional comments (1)
example/src/main/java/example/chat/StreamChatExample.java (1)
57-59: Review comment is based on incorrect assumption about null-safety for different event types.The review suggests adding a null-check to
CONVERSATION_MESSAGE_DELTAsimilar toCONVERSATION_CHAT_COMPLETED. However, examination of the PR and codebase shows:
- The PR specifically addresses null-safety for
CONVERSATION_CHAT_COMPLETEDevents only (line 60-62)- All four example files in the codebase (
StreamChatExample.java,ChatWithImageExample.java,SubmitToolOutputExample.java,StreamWorkflowChatExample.java) consistently useevent.getMessage().getContent()without null-checks forCONVERSATION_MESSAGE_DELTAevents- The API documentation indicates
CONVERSATION_MESSAGE_DELTAis for "incremental message...usually when type=answer", suggesting the message field is guaranteed to be presentThe null-safety issue addressed in the PR appears specific to
CONVERSATION_CHAT_COMPLETEDevents, notCONVERSATION_MESSAGE_DELTA. Adding defensive checks toCONVERSATION_MESSAGE_DELTAis therefore unnecessary and would diverge from the established pattern across all examples.Likely an incorrect or invalid review comment.
| } | ||
| if (ChatEventType.CONVERSATION_CHAT_COMPLETED.equals(event.getEvent())) { | ||
| if (MessageType.FOLLOW_UP.equals(event.getMessage().getType())) { | ||
| if (event.getMessage() != null && MessageType.FOLLOW_UP.equals(event.getMessage().getType())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct NPE fix, but address the formatting failure.
The null-check correctly prevents the NullPointerException described in the PR. The logic properly handles both cases: when a message exists with type FOLLOW_UP, and when the message is null (falling through to print token usage).
However, the pipeline indicates a Spotless formatting violation. Run mvn spotless:apply to fix the code style.
🤖 Prompt for AI Agents
In example/src/main/java/example/chat/StreamChatExample.java around line 61 the
null-check for event.getMessage() fixes the NPE but the file fails Spotless
formatting; run mvn spotless:apply (or your project's formatter) and commit the
formatted changes so the formatting violation is resolved.
fix: When chat event type is conversation.chat.completed, a NPE will be thrown;
Error occurred: Cannot invoke "com.coze.openapi.client.connversations.message.model.Message.getType()" because the return value of "com.coze.openapi.client.chat.model.ChatEvent.getMessage()" is null