Skip to content

fix(client-python): clean pending DAP request on send failure#736

Open
wdwd720 wants to merge 2 commits intorocketride-org:developfrom
wdwd720:fix/python-dap-send-failure-cleanup
Open

fix(client-python): clean pending DAP request on send failure#736
wdwd720 wants to merge 2 commits intorocketride-org:developfrom
wdwd720:fix/python-dap-send-failure-cleanup

Conversation

@wdwd720
Copy link
Copy Markdown

@wdwd720 wdwd720 commented May 1, 2026

Summary

  • Clean up Python DAP pending request state when transport send fails.
  • Prevent stale pending request futures after failed sends.
  • Add focused regression coverage with a fake transport.

Type

fix

Testing

  • Tests added or updated
  • Tested locally
  • ./builder test passes

Checklist

  • Commit messages follow conventional commits
  • No secrets or credentials included
  • Wiki updated (if applicable)
  • Breaking changes documented (if applicable)

Linked Issue

Fixes #735

Problem

DAPClient.request() registers a pending request before sending. If the send path raises, the method exits before the normal pending-request cleanup runs, leaving stale state behind.

Fix

Remove the request's pending entry when send fails, while preserving the existing exception behavior.

Successful request handling, timeout behavior, response correlation, and disconnect cleanup are unchanged.

Local Testing

  • PYTHONDONTWRITEBYTECODE=1 python3 -m pytest packages/client-python/tests/test_dap_client.py -q
  • PYTHONDONTWRITEBYTECODE=1 python3 -m pytest packages/client-python/tests/test_dap_client.py packages/client-python/tests/test_client_env_loading.py -q

Notes

This PR is intentionally scoped to the Python client DAP send-failure path.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed issue where pending requests were not properly cleaned up when transport send operations failed, preventing stale pending state and potential resource leaks.
  • Tests

    • Added comprehensive test coverage for transport send failure scenarios, verifying proper error conversion and internal state cleanup.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

📝 Walkthrough

Walkthrough

The request method in DAPClient now explicitly cleans up pending request entries from _pending_requests when transport send fails, preventing stale request correlation state. A test verifies the fix with a mock transport that always fails on send.

Changes

Cohort / File(s) Summary
Bug Fix
packages/client-python/src/rocketride/core/dap_client.py
Added explicit removal of the correlation entry (seq) from _pending_requests in the exception handler when transport send fails, preventing stale pending state from persisting.
Test Coverage
packages/client-python/tests/test_dap_client.py
New async test that verifies send failure handling by using a mock transport that always raises on send, confirming the error converts to ConnectionError, send is called once, and _pending_requests is empty after failure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A pending request would linger and stay,
When send failed, it had no way to decay.
But now with a cleanup so neat and so grand,
The stale entries vanish—hooray, how they're banned! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: cleaning up pending DAP requests when transport send fails, which matches the core objective.
Linked Issues check ✅ Passed The PR fully addresses issue #735 by removing pending request entries on send failure, implementing cleanup logic and regression tests that validate the fix.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the send-failure cleanup in Python DAP client; no unrelated modifications are present.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the module:client-python Python SDK and MCP client label May 1, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

No description provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:client-python Python SDK and MCP client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python DAP request leaks pending entry when transport send fails

2 participants