Skip to content

[lldb-dap] Adding exception handling for dap server disconnect and terminations in lldbdap_testcase.py #155335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

[lldb-dap] Adding exception handling for dap server disconnect and terminations in lldbdap_testcase.py #155335

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
443c2b4
Improving lldbdap server error diagnosability and adding exception ha…
Aug 26, 2025
1c68d3f
spliting into 2 PRs and consolidating exeptions
Aug 26, 2025
6a2c9a7
Using addTearDownHook and formatting
Aug 26, 2025
8795455
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 27, 2025
4765008
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 27, 2025
ca1e606
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 27, 2025
dbb55a9
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 27, 2025
de5efe2
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 27, 2025
e36edb7
Reverting to original format
Aug 28, 2025
0cedbbf
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 28, 2025
68eddcb
Format document
Aug 28, 2025
3f0102d
Merge branch 'main' into lldbdap
piyushjaiswal98 Aug 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,15 @@ def attach(
# if we throw an exception during the test case.
def cleanup():
if disconnectAutomatically:
self.dap_server.request_disconnect(terminateDebuggee=True)
self.dap_server.terminate()
try:
Copy link
Contributor

@ashgti ashgti Aug 26, 2025

Choose a reason for hiding this comment

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

We could add both of these as two distinct cleanups. Exceptions thrown in a addTearDownHook are logged already, but I think the goal of this change is to ensure the .terminate is called. The hooks are called in FIFO order, so we could change this to:

if disconnectAutomatically:
  self.addTearDownHook(lambda: self.dap_server.request_disconnect(terminateDebuggee=True))
self.addTearDownHook(lambda: self.dap_server.terminate())

Copy link
Contributor Author

@piyushjaiswal98 piyushjaiswal98 Aug 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the suggestion. I think adding both as separate tear down hooks makes sense. I'll update the PR

Copy link
Contributor Author

@piyushjaiswal98 piyushjaiswal98 Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

On investigation...addTearDownHook appears to follow a LIFO order...This approach doesn't seem to work... falling back to the original

self.dap_server.request_disconnect(terminateDebuggee=True)
except (ValueError, TimeoutError, BrokenPipeError, ConnectionError, Exception) as e:
# DAP server might not be responsive, skip disconnect and terminate directly
print(f"Warning: disconnect failed ({e}), skipping and terminating directly")
try:
self.dap_server.terminate()
except Exception as e:
print(f"Warning: terminate failed ({e}), DAP server may have already died")

# Execute the cleanup function during test case tear down.
self.addTearDownHook(cleanup)
Expand All @@ -477,9 +484,20 @@ def cleanup():
if expectFailure:
return response
if not (response and response["success"]):
self.assertTrue(
response["success"], "attach failed (%s)" % (response["message"])
)
error_msg = "attach failed"
if response:
if "message" in response:
error_msg += " (%s)" % response["message"]
elif "body" in response and "error" in response["body"]:
if "format" in response["body"]["error"]:
error_msg += " (%s)" % response["body"]["error"]["format"]
else:
error_msg += " (error in body)"
else:
error_msg += " (no error details available)"
else:
error_msg += " (no response)"
self.assertTrue(response and response["success"], error_msg)

def launch(
self,
Expand Down