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

Ensure that the Debug API is working with the new RPC #11268

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

tortmayr
Copy link
Contributor

@tortmayr tortmayr commented Jun 7, 2022

What it does

After migrating to the new binary RPC protocol (83d4308), debugging is no longer working as expected.
The main reason for this an incomplete migration of the Debug API to the new binary message channels. In addition to being incomplete, this refactoring is also unnecessary. The Debug (Adapter) Protocol is completely string based anyway. Thus, there is no benefit in migrating to the binary protocol. So this PR fixes Debugging in Theia by

  • Restoring the original string-based message channel for Debug communication.
  • Renaming the string-based Channel used in the Debug API to DebugChannel to avoid naming confusion with the default binary message Channels.
  • Only use binary channels where absolutely necessary and introduce a ForwardingDebugChannel which can be used to wrap an arbitrary binary message Channel (i.e. the service websocket channel) in a DebugChannel

Fixes #11263

Contributed on behalf of STMicroelectronics

How to test

  1. Open a .spec.ts file in the repo and try the Run Mocha Tests debug configuration.
  2. Debug view should work as expected. (Variables are shown, debug, debug trace, breakpoins are hit etc.
  3. Try a dynamic Node configuration like 'Run current file'. Should also work as expected.

Review checklist

Reminder for reviewers

After migrating to new binary RPC (eclipse-theia@83d4308) Debugging was no longer working as expected.
- Partly revert/Switch back to use a string-based message channel for Debug communication. 
- Rename the string-based `Channel` used in the Debug API to `DebugChannel` to avoid naming confusion with the default binary message `Channel`s.
- Introduce a `ForwardingDebugChannel` which can be used to wrap an arbitrary binary message `Channel` (i.e. the service websocket channel` in a `DebugChannel`

Fixes eclipse-theia#11263

Contributed on behalf of STMicroelectronics
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

With these changes, the variety of debug sessions that I can test all run correctly. 👍 Thanks!

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

The changes also look good to me 👍

@JonasHelming JonasHelming merged commit 57f6415 into eclipse-theia:master Jun 9, 2022
@JonasHelming JonasHelming added this to the 1.27.0 milestone Jun 9, 2022
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.

Debug sessions failing to execute
4 participants