Skip to content

Conversation

qevolg
Copy link
Contributor

@qevolg qevolg commented Sep 8, 2025

Description

Fix closing ws connection

Jira: https://jira.taosdata.com:18080/browse/TD-37771

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

@qevolg qevolg requested a review from Copilot September 8, 2025 08:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes WebSocket connection handling by addressing a circular reference issue that prevented proper connection cleanup when WsTaos instances are dropped.

  • Changes the connection management function to use a weak reference to WsTaos instead of a strong reference
  • Adds a test to verify that dropping a WsTaos instance properly closes the WebSocket connection
  • Adds proper state management in the WsTaos Drop implementation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
taos-ws/src/query/conn.rs Modified run function to accept Weak<WsTaos> parameter and added safety checks when accessing WsTaos methods; added new test for connection cleanup
taos-ws/src/query/asyn.rs Updated to pass weak reference to connection handler and added state cleanup in Drop implementation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 95.87629% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.57%. Comparing base (0c9b05a) to head (7d8905e).

Files with missing lines Patch % Lines
taos-ws/src/query/conn.rs 95.78% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #501      +/-   ##
==========================================
- Coverage   82.63%   82.57%   -0.07%     
==========================================
  Files         117      117              
  Lines       56703    56790      +87     
==========================================
+ Hits        46857    46892      +35     
- Misses       9846     9898      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qevolg qevolg requested a review from zitsen September 9, 2025 02:55
@zitsen zitsen merged commit b9a6a6e into main Sep 17, 2025
5 checks passed
@zitsen zitsen deleted the fix/TD-37771 branch September 17, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants