| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sonar python:S7497: connect_with_backoff() caught CancelledError during the reconnect-delay wait but swallowed it with `pass`. A parent-driven cancellation (e.g., process shutdown) could be lost and the outer `while True` reconnect loop would continue. Replace `pass` with `raise` to propagate the cancellation. The preceding lines already perform local cleanup (cancelling helper tasks, clearing retry_event), so no additional cleanup is needed before re-raising. The similar handler at line 312 is a different pattern (cancel-then- await on a child task this code just cancelled); it intentionally suppresses the expected CancelledError from the awaited task and is addressed in a separate finding.
…agates Sonar python:S7497: the cleanup loop after asyncio.wait() cancelled each pending task and awaited it inside `try/except CancelledError: pass`. The handler swallowed the expected CancelledError from the just-cancelled child, but it would also swallow a CancelledError from the OUTER `connect_with_backoff` coroutine if the parent arrived mid-await -- breaking shutdown propagation. Cancel all pending tasks first, then drain them via `asyncio.gather(..., return_exceptions=True)`. Per-child CancelledError is collected as a result and ignored; non-cancellation exceptions are re-raised. If `connect_with_backoff` itself is cancelled while awaiting the gather, gather propagates the cancellation unconditionally. Side-benefit: the original sequential loop would skip cancelling later tasks if an earlier task raised non-CancelledError. The gather form cancels all up front, so cleanup is complete regardless of outcome.
Cancel both helper tasks and clear the retry event before re-raising asyncio.CancelledError so reconnect-delay cancellation does not leave dangling state behind.
|
Sorry, something went wrong.
|
@cubic-dev-ai review this PR |
Sorry, something went wrong.
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
Sorry, something went wrong.
There was a problem hiding this comment.
Improves asyncio cancellation handling in the MCP stdio-python bridge to avoid leaving background tasks running across reconnect/shutdown paths.
Changes:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Algorithm received good improvements. LGTM!
Sorry, something went wrong.
* nd-mcp: re-raise asyncio.CancelledError in reconnect-delay handler Sonar python:S7497: connect_with_backoff() caught CancelledError during the reconnect-delay wait but swallowed it with `pass`. A parent-driven cancellation (e.g., process shutdown) could be lost and the outer `while True` reconnect loop would continue. Replace `pass` with `raise` to propagate the cancellation. The preceding lines already perform local cleanup (cancelling helper tasks, clearing retry_event), so no additional cleanup is needed before re-raising. The similar handler at line 312 is a different pattern (cancel-then- await on a child task this code just cancelled); it intentionally suppresses the expected CancelledError from the awaited task and is addressed in a separate finding. * nd-mcp: drain cancelled tasks via gather() so outer cancellation propagates Sonar python:S7497: the cleanup loop after asyncio.wait() cancelled each pending task and awaited it inside `try/except CancelledError: pass`. The handler swallowed the expected CancelledError from the just-cancelled child, but it would also swallow a CancelledError from the OUTER `connect_with_backoff` coroutine if the parent arrived mid-await -- breaking shutdown propagation. Cancel all pending tasks first, then drain them via `asyncio.gather(..., return_exceptions=True)`. Per-child CancelledError is collected as a result and ignored; non-cancellation exceptions are re-raised. If `connect_with_backoff` itself is cancelled while awaiting the gather, gather propagates the cancellation unconditionally. Side-benefit: the original sequential loop would skip cancelling later tasks if an earlier task raised non-CancelledError. The gather form cancels all up front, so cleanup is complete regardless of outcome. * nd-mcp: address PR-review findings Cancel both helper tasks and clear the retry event before re-raising asyncio.CancelledError so reconnect-delay cancellation does not leave dangling state behind. --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud> (cherry picked from commit 735549f)
| Back | FazBrowse Home | New Git URL |
Summary