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

Ignore non-standard DAP events to ensure debugging can start #12669

Closed
wants to merge 1 commit into from

Conversation

pieterdd
Copy link

@pieterdd pieterdd commented Jan 25, 2025

As per #11777, this issue caused Helix to stop debugging when launching a debugpy DAP session:

2024-09-25T15:40:33.914 helix_dap::transport [ERROR] err: <- Parse(Error("unknown variant debugpySockets, expected one of initialized, stopped, continued, exited, terminated, thread, output, breakpoint, module, loadedSource, process, capabilities, memory", line: 0, column: 0))

This fix allows a debugpy DAP session to start up. Although there are other issues preventing debugpy to be used in conjunction with Helix, this is a step forward.

@the-mikedavis
Copy link
Member

I don't think we should be discarding all parsing failures. If the Event type is flexible in what it accepts like Request/Response it should be written as a trait instead of an enum. It's very similar in the spec to LSP's notification type which is also written as a trait (helix-lsp-types/src/notification.rs).

@the-mikedavis
Copy link
Member

I've pushed up fec5101 which moves the helix_dap code closer to what we have in helix_lsp, including logging and discarding unknown events. I tried the reproduction case from #11777 and see the same behavior between these changes and mine - discarding the notifications gets further in the DAP session but it still crashes out - so I will close this out.

@pieterdd pieterdd deleted the python-dap-issues branch January 27, 2025 21:04
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.

3 participants