Dev/issues 3#242
Closed
lzsweb wants to merge 11 commits into
Closed
Conversation
added 2 commits
July 2, 2026 10:05
- checkStatus() now also checks isOpen to detect unconnected state - onFailure() sets isClosed=true and isOpen=false for consistency - onClosed() sets isClosed=true to match onClosing() - close() is now idempotent via compareAndSet and null-safe - sendMessage() throws on send failure instead of silently ignoring - connect() skips isOpen check to allow initial connection - onClosing() delegates to close() to avoid duplicated logic
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request bumps the SDK version to 2.22.24 and refactors connection state management in OmniRealtimeConversation.java, including atomic state transitions during close operations and stricter status checks. Feedback focuses on improving robustness: first, connect() should prevent re-connection if already open to avoid resource leaks; second, both onClosed() and onFailure() should count down the disconnectLatch to prevent threads waiting in endSession() from blocking until a timeout.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…ndSession latch on disconnect - connect() now rejects re-connection when already open to avoid resource leaks - onClosed() and onFailure() count down disconnectLatch so endSession() does not block until timeout when the connection is lost
When the server encrypts output without setting X-DashScope-OutputEncrypted header, the SDK previously assumed output was always a JSON object and threw on string-type values. Now both the encrypted and non-encrypted paths inspect the actual JSON type before casting, preventing IllegalStateException and UnsupportedOperationException.
…ncrypted header - Add fallback decryption in DashScopeResult.fromResponse() 4-arg path: when server encrypts output but does not set the encryption header, detect output as a JSON string and decrypt using the request's encryption config (AES key + IV) - Add instanceof JsonObject guards in all fromDashScopeResult() methods (Generation, MultiModalConversation, ImageGeneration, ImageSynthesis, Conversation, Application) to prevent ClassCastException when output is not a JSON object, logging the issue instead
When server returns encrypted output but client didn't enable encryption, throw clear ApiException instead of letting encrypted string leak to downstream parsers causing fastjson syntax errors. Detection logic: output is JsonPrimitive, length > 100, matches base64 pattern. Exception message guides users to set enableEncrypt(true). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The 3-arg fromResponse method with isFlattenResult=true was missing encryption detection, causing fastjson parse errors when server returned encrypted base64 output. Added same defensive check as the non-flatten path to throw clear EncryptionMismatch exception. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ersation DashScopeResult: - Restore original isFlatten behavior: output = entire JSON object (not just output field) - Remove heuristic encrypted-output detection (length>100 + base64 regex) that caused false positives - Extract parseOutputField() to unify output parsing logic across 3 code paths - Fallback decryption only triggers when req.getEncryptionConfig() != null OmniRealtimeConversation: - Replace volatile closeCode/closeReason with AtomicReference<CloseInfo> for atomic updates - onFailure now calls callback.onClose(-1, "failure:" + msg) to notify downstream - onClosed also records closeInfo for consistency Tests (23 new, all passing): - TestDashScopeResult: output type-safety (JsonObject/JsonPrimitive/JsonNull/absent), isFlatten behavior, encryption fallback with/without header, no-false-positive when config null - TestOmniRealtimeConversation: close() idempotency, onFailure callback, onClosing checkStatus, connect() state guards, onClosed callback, disconnectLatch release - TestResultTypeSafety: Result subclasses handle JsonPrimitive output gracefully
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.