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.

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

Jump to bottom

Improve debugger stability #457

Merged
merged 13 commits into from
Jan 14, 2021
Merged

Improve debugger stability #457

merged 13 commits into from
Jan 14, 2021

Conversation

lukaszsamson
Copy link
Collaborator

This PR introduces stability and reliability improvements to debugger.

  • I added some safe checking in the debugger code
  • Made the server not crash when handling invalid request (e.g. when threadId, variablesReference or frameId is not found)
  • Adds handling of :int module crashes. Some operations crash with MatchError when the operation results in a step out of interpreted code
  • Made debugger aware of process exits. If a paused process dies now the debugger is notified and able to send thread event with reason exited. vscode UI is no longer stuck in paused state
  • Refactored and added tests to OutputDevice. The module now now more closely adheres to erlang I/O protocol (see https://erlang.org/doc/apps/stdlib/io_protocol.html). Most importantly:
    • handling of encoding was broken - no care was taken to check if data is unicode or latin1
    • in case of multiple requests the protocol expects only one response and that processing is stopped on first error - all responses was returned

Fixes #452
Fixes #454
Fixes #455
Fixes #456

@lukaszsamson
remove legacy io_request handlers
01c8bee 
we don't support OTP < R15B
@lukaszsamson
rescue MatchError in :int calls
ec5fdda 
Fixes #455
@lukaszsamson
make output device better conform to erlang I/O protocol
081065b 
see https://erlang.org/doc/apps/stdlib/io_protocol.html for details
@lukaszsamson
return WireProtocol.send error to the caller
1d74fc7 
no need to IO.warn if write fails
@lukaszsamson
we are redirection stderr to stdout, use stdout as underlying device
c5cec56
@lukaszsamson
inspect error
676a666
@lukaszsamson
monitor debugged processes
9bf544a 
add test for mix task exit
Fixes #454
@lukaszsamson
avoid debugger crashes when handling requests for no longer existing …
b6f2102 
…thread, frame and variable ids

Fixes #452
@lukaszsamson
add test
bd38e73
@lukaszsamson
forbid changes of underlying device opts
7a868e9
@lukaszsamson
refactor and add tests coverage to invalid requests
cc7fb86
@lukaszsamson
Map.pop! is available since elixir 1.10
07a6718
@lukaszsamson
run formatter
9278ede
axelson
axelson approved these changes Jan 14, 2021
View reviewed changes
Copy link
Member

@axelson axelson left a comment

Choose a reason for hiding this comment

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

That is an impressive list of fixes ❤️

apps/elixir_ls_debugger/lib/debugger/output.ex
@@ -7,7 +7,7 @@ defmodule ElixirLS.Debugger.Output do
are sent with sequence numbers that are unique and sequential, and includes client functions for
sending these messages.
"""
import ElixirLS.Utils.WireProtocol, only: [send: 1]
alias ElixirLS.Utils.WireProtocol
Copy link
Member

Choose a reason for hiding this comment

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

👍 💯

@axelson axelson merged commit 4ae6e22 into master Jan 14, 2021
@axelson axelson deleted the ls-improve-debugger-stability branch January 14, 2021 18:21
lukaszsamson added a commit that referenced this pull request Apr 20, 2023
@lukaszsamson
correctly encode output messages as binary
18aedca 
Fixes elixir-lsp/vscode-elixir-ls#322
Bug introduced in #457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axelson axelson axelson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@lukaszsamson @axelson