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

[Breakpoints] Leftover breakpoint breaks the debugger #3682

Closed
eoger opened this issue Aug 16, 2017 · 8 comments
Closed

[Breakpoints] Leftover breakpoint breaks the debugger #3682

eoger opened this issue Aug 16, 2017 · 8 comments

Comments

@eoger
Copy link

eoger commented Aug 16, 2017

I set a breakpoint in a previous session in the browser toolbox debugger.
When I open the debugger (even after restarting the browser multiple times), clicks on the Firefox UI are not registered and CSS :hover rules are not applied.
Trying to remove that breakpoint triggers an error.

Screencast:
https://gfycat.com/gifs/detail/JubilantDeepAyeaye

Entire screenshot of the errors in the screencast:
screen shot 2017-08-16 at 10 45 00 am

cc @vladikoff

@jasonLaster
Copy link
Contributor

jasonLaster commented Aug 16, 2017

@eoger mind sharing the STR? I'd like to see if i can reproduce this and see what's going on

@vladikoff
Copy link

@eoger Was this in Nightly?

@eoger
Copy link
Author

eoger commented Aug 16, 2017

Yes this was Nightly

@eoger
Copy link
Author

eoger commented Aug 16, 2017

@jasonLaster wasn't able to reproduce exactly. But:

  • Open Firefox
  • Open browser toolbox, set a breakpoint in browser-sync, line 260.
  • Cmd+Q, reopen browser, reopen debugger

From then the breakpoint has disappeared, but I can't set it up again (breakpoint disappears after I click):
https://gfycat.com/gifs/detail/LavishEnviousAdder
screen shot 2017-08-16 at 11 57 20 am

@vladikoff
Copy link

Ok I was able to reproduce this locally, the breakpoint won't stick to the line number:

action-s

@vladikoff
Copy link

STR

  • Fresh Nightly Profile (57.0a1 (2017-08-16) (64-bit))
  • Open toolbox
  • Set a breakpoint at browser-sync.js by going to Debugger, looking for file called browser-sync.js:

image

  • CMD+Q DevTools, CMD+Q Firefox
  • Restart the created profile
  • Open Toolbox, browser-sync.js is open
  • Scroll to line, no breakpoint set:

image

  • Try to set a breakpoint on that line:
    260

Errors in console:

image

@jasonLaster jasonLaster changed the title Leftover breakpoint breaks the debugger [Breakpoints] Leftover breakpoint breaks the debugger Aug 23, 2017
@codehag codehag added this to the September 4th milestone Aug 23, 2017
@jasonLaster
Copy link
Contributor

I believe this bug is coming the fact that we "move" breakpoints to keep them in the same original location when the user reloads.

Scenario:

  1. the app is using sourcemaps to bundle three files (a,b,c).
  2. The user adds a breakpoint to file b, line 10
  3. the user changes file a
  4. the user reloads the page
  5. the debugger sees that the file b, line 10 breakpoint points to the wrong generated location
  6. the debugger "moves" the breakpoint to the new location, which means
    6.a it updates the original locations in the UI
    6.b it creates a new breakpoint on the server
    6.c it forgets to remove the old breakpoint on the server

This diff is a sketch of what i think the solution should look like. When we see the new and old generated location is different, we remember to clean up after ourselves and remove the breakpoint.

diff --git a/src/actions/breakpoints/syncBreakpoint.js b/src/actions/breakpoints/syncBreakpoint.js
index ee220b4..4237afb 100644
--- a/src/actions/breakpoints/syncBreakpoint.js
+++ b/src/actions/breakpoints/syncBreakpoint.js
@@ -64,6 +64,10 @@ export async function syncClientBreakpoint(
       newGeneratedLocation
     );

+    if (locationMoved(generatedLocation, newGeneratedLocation)) {
+      await client.removeBreakpoint(generatedLocation);
+    }
+
     const breakpoint = {
       ...pendingBreakpoint,
       id: makeLocationId(newLocation),
diff --git a/src/client/firefox/commands.js b/src/client/firefox/commands.js
index 40f2232..e405e9d 100644
--- a/src/client/firefox/commands.js
+++ b/src/client/firefox/commands.js
@@ -125,9 +125,9 @@ function setBreakpoint(
     });
 }

-function removeBreakpoint(breakpoint: Breakpoint) {
+function removeBreakpoint(generatedLocation) {
   try {
-    const id = makeLocationId(breakpoint.generatedLocation);
+    const id = makeLocationId(generatedLocation);
     const bpClient = bpClients[id];
     if (!bpClient) {
       console.warn("No breakpoint to delete on server");

This was referenced Aug 26, 2017
@jasonLaster
Copy link
Contributor

This is fixed here:

https://bugzilla.mozilla.org/show_bug.cgi?id=1394495

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

No branches or pull requests

4 participants