Skip to content
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.

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

[macOS] Properly handle cases where lldb-dap cannot be found with xcrun #1119

Merged
merged 10 commits into from
Oct 4, 2024

Conversation

matthewbastien
Copy link
Member

@matthewbastien matthewbastien commented Oct 2, 2024

Some users were seeing the extension fail to activate due to lldb-dap not being found in the toolchain. I've tracked this down to three underlying issues with the extension:

  1. The DebugAdapter.verifyDebugAdapterExists() function was awaited on extension startup
  2. The SwiftToolchain.getLLDB() and SwiftToolchain.getLLDBDebugAdapter() functions can throw errors when the executables are not found. This was not being caught properly by DebugAdapter.verifyDebugAdapterExists().
  3. There are inconsistencies between DebugAdapter.adapterName and DebugAdapter.getAdapterType() that can cause them to return different debug adapters under certain circumstances.

The DebugAdapter.verifyDebugAdapterExists() will now catch errors thrown by the SwiftToolchain when finding executables and the extension activation now properly runs this check in the background. I've also updated the docstrings for getLLDB() and getLLDBDebugAdapter() to make it more obvious that they can throw and added some tests in the hopes that this won't happen again.

I've replaced the DebugAdapter.adapterName property and DebugAdapter.getAdapterType() function with a single DebugAdapter.getLaunchConfigType() function. This will hopefully reduce any further issues with adapter mismatches from happening in the future.

I've added mock-fs as a dependency now since these new tests rely on a lot of file system checks and it's easier to mock the fs module this way. The documentation for writing tests has been updated to reflect this.

Related issues: #1069 #1077

src/debugger/debugAdapter.ts Outdated Show resolved Hide resolved
src/debugger/debugAdapterFactory.ts Outdated Show resolved Hide resolved
@matthewbastien matthewbastien merged commit b69ee94 into swiftlang:main Oct 4, 2024
8 checks passed
@matthewbastien matthewbastien deleted the failed-to-find-lldb-dap branch October 8, 2024 14:50
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