Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

fix: when a setBreakpoint request comes in, always respond only once #688

Merged
merged 1 commit into from
Jan 1, 2021

Conversation

jvilk-stripe
Copy link
Contributor

@jvilk-stripe jvilk-stripe commented Dec 23, 2020

Description of change and why it was needed here

Fixes #510 and #657

This fixes a notorious bug that engineers at Stripe encounter daily: The debugger fails to attach because they have a breakpoint that is unset in a file.

Specifically, the situation occurs when VS Code has one file open that has all unset breakpoints (e.g., there are no active breakpoints in the file). It turns out that VS Code sends out a setBreakpoints request with no breakpoints for this file. In this case, the Ruby debug adapter's code would not send a response to this request, so VS Code would wait forever for a response that would never come.

I've updated the code to ensure that it always responds once to this sort of request, and fixes another bug where I think it might respond twice to a single setBreakpoints request.

  • [] The build passes
  • [] TSLint is mostly happy
  • [] Prettier has been run: I did not run prettier because the code I edited has never been prettiered, and I did not want to dirty the diff.

@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #688 (2c53519) into master (e2f901c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #688   +/-   ##
======================================
  Coverage    5.08%   5.08%           
======================================
  Files          10      10           
  Lines         118     118           
  Branches       20      20           
======================================
  Hits            6       6           
  Misses        112     112           
Flag Coverage Δ
language_server_ruby 5.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2f901c...2c53519. Read the comment docs.

@@ -50,7 +50,7 @@ export function spawn<T = string>(
if (typeof b === 'string') {
chunk = b.toString();
} else {
chunk = b.toString(optsWithoutStdIn.encoding || 'utf8');
chunk = b.toString(<BufferEncoding> optsWithoutStdIn.encoding || 'utf8');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason this code wouldn't typecheck / build w/o this cast.

@jvilk-stripe
Copy link
Contributor Author

@wingrunr21 Hello again friend! Fixing this bug would be really impactful for us at Stripe. Please let me know if there's anything I can do to make the review process easier for you.

@wingrunr21 wingrunr21 merged commit 9415340 into rubyide:master Jan 1, 2021
@jvilk-stripe
Copy link
Contributor Author

@wingrunr21 exciting! 🎉 do you have plans for a release w/ this fix anytime soon? looks like the last vscode-ruby release was in March or so.

@wingrunr21
Copy link
Collaborator

Yes, I'm doing a few dependency updates and will get it cut. Today/tomorrow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Launching with the debugger does not work if there are disabled breakpoints
2 participants