chore: tidy spring websocket client files #448
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| return () -> { | ||
| IOException ioFailure = null; | ||
| Exception nonIoFailure = null; | ||
| try { | ||
| client.send(new CloseMessage(subscriptionId)); |
There was a problem hiding this comment.
Calling client.send(new CloseMessage(subscriptionId)) in the unsubscribe path uses StandardWebSocketClient.send, which always waits for a relay response or a 60s timeout before returning. Relays are not required to reply to a NIP-01 CLOSE, so cancelling a subscription can block the caller for the full timeout per relay before the socket is closed. This turns an otherwise asynchronous cancel into a minute-long pause and will accumulate if multiple subscriptions are torn down. Send the CLOSE frame without the blocking send() loop or close the socket first so teardown returns promptly.
Useful? React with 👍 / 👎.
Qodana Community for JVM127 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at [email protected]
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| try { | ||
| client.send(new CloseMessage(subscriptionId)); | ||
| } catch (IOException e) { | ||
| safeError.accept(e); | ||
| ioFailure = e; | ||
| } | ||
|
|
||
| try { | ||
| delegate.close(); |
There was a problem hiding this comment.
The returned AutoCloseable sends a CloseMessage via client.send(...) before removing the listener. SpringWebSocketClient.send ultimately calls StandardWebSocketClient.send, which blocks until a relay message arrives or the 60‑second await timeout elapses and then forcibly closes the session. Nostr relays are not required to send any response to a CLOSE command, so calling close() on the subscription handle will typically hang for up to the await timeout and may tear down the socket even though the caller only wanted to unsubscribe. The close sequence should transmit the CLOSE frame without waiting for a reply (e.g., using the asynchronous subscribe transport) so that subscription cleanup is immediate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Sent CLOSE frames via the non-blocking subscription transport to prevent unsubscribe calls from waiting on
StandardWebSocketClient.sendtimeouts. nostr-java-api/src/main/java/nostr/api/WebSocketClientHandler.javaL124-L185 - Tidied the teardown path to immediately dispose of the temporary listener handle and preserve the first error encountered while shutting down subscription resources. nostr-java-api/src/main/java/nostr/api/WebSocketClientHandler.javaL124-L185
Testing
- ❌
./mvnw -q -T1 verify(fails: Maven wrapper download cannot reach repo.maven.apache.org through the proxy)
- Updated the decode method to ensure non-blocking behavior during subscription closure. This change improves the responsiveness of the API and enhances user experience.
- Reintroduced the @OverRide annotation for the decode method to enhance code clarity and maintain consistency with Java best practices.
- Clarified parameter descriptions and return values for the encode and decode methods in the Bech32 class. - Enhanced exception handling details to provide better guidance on potential errors.
- Changed classTypeTagsMap to be final to prevent reassignment. - This enhances code safety and clarity regarding the intended usage.
- Clarify parameters and return values for the sign and verify methods. - Improve overall understanding of the Schnorr signature process.
- The id field was commented out and is no longer needed. - This change simplifies the class structure and improves clarity.
…alidation - Simplified exception handling by removing the redundant catch block for AssertionError. This improves code clarity and maintains the existing validation logic.
- Added the @OverRide annotation back to the decode method for clarity. - This improves code readability and ensures proper method overriding.
- Reintroduces the @OverRide annotation for the decode method to ensure proper method overriding and improve code clarity.
- Added a suppression annotation to avoid resource warnings when creating an HttpClient instance in the Nip05Validator class.
…entation - Added missing override annotations to several methods for clarity. - Cleaned up method documentation by removing redundant comments. - Improved code readability and maintainability.
- Updated relayName and relayUri to be final, ensuring they are immutable. - This change enhances the integrity of the WebSocketClientHandler class.
- Streamlined the process of adding tags by using computeIfAbsent to reduce code complexity and improve readability.
Summary
Related issue: #____
What changed?
java.io.IOExceptionimport inSpringWebSocketClientto keep the implementation clean and warning-free.BREAKING
None.
Review focus
Checklist
Testing
mvn -q verify(fails because.mvn/maven.configinjects an invalid-T 2threads flag in this environment)Network Access
Notes
-T1if a full build is required.https://chatgpt.com/codex/tasks/task_b_68e008f70d6083319efb6ff3f85b9c1e