Closed Bug 821610 Opened 11 years ago Closed 6 years ago

Break on XHR

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(firefox64 fixed)

RESOLVED FIXED
Future
Tracking Status
firefox64 --- fixed

People

(Reporter: vporof, Assigned: loganfsmyth)

References

(Blocks 3 open bugs)

Details

(Keywords: dev-doc-complete, Whiteboard: [polish-backlog])

Attachments

(2 files, 1 obsolete file)

      No description provided.
We need a way to break on XHR requests (POST, GET, etc.)
take a look at firebug's implementation. Or we could ask Honza!

filter on BLACKEAGLE.
Priority: -- → P2
Honza, how do you do this? Can you point us at the relevant code in Firebug?
Flags: needinfo?(odvarko)
1) First you need to handle "http-on-opening-request" (nsIObserver). Note that it was "http-on-opening-request" in the past (Fx 16), but that event is not synchronous anymore (see https://bugzilla.mozilla.org/show_bug.cgi?id=800799)

Here is where Firebug does it:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/net/netMonitor.js#L404

2) As soon as the handler is executed (synchronously), the debugger needs to check if there is an enabled breakpoint and associated with the URL.

https://github.com/firebug/firebug/blob/master/extension/content/firebug/net/netProgress.js#L224

3) If there is a breakpoint, optional condition needs to be evaluated. The condition should be able to deal at least with URL params and post data.

https://github.com/firebug/firebug/blob/master/extension/content/firebug/net/netProgress.js#L229

4) If the breakpoint says: "break", the debugger needs the interrupt JS execution synchronously (to keep the current stack trace).

https://github.com/firebug/firebug/blob/master/extension/content/firebug/net/netProgress.js#L256

5) Consequently, the debugger needs to peel off the stack trace and keep only frames coming from the content (web page).

Up to now all happens on the server side. Now, the server should just send 'pause' event to the client and the client should handle it just like it handles other interrupts...

See this posts for UX info:
http://www.softwareishard.com/blog/firebug/firebug-tip-conditional-breakpoints/
http://www.softwareishard.com/blog/firebug/firebug-15-xhr-breakpoints/


Honza
Flags: needinfo?(odvarko)
This is excellent! Thanks!
(In reply to Jan Honza Odvarko from comment #4)
> 1) First you need to handle "http-on-opening-request" (nsIObserver). Note
> that it was "http-on-opening-request" in the past (Fx 16), but that event is
> not synchronous anymore (see
Just a little correction, it was "http-on-modify-request" in the past...

Honza
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Filed bug 832983.
Depends on: 832983
It doesn't look like I'll be working on this in the near future. Unassigning, for now.
Assignee: vporof → nobody
Status: ASSIGNED → NEW
Summary: Break on XHR → Break on XHR (frontend)
Summary: Break on XHR (frontend) → Break on XHR
Whiteboard: [devedition-40]
Whiteboard: [devedition-40] → [polish-backlog]
I've just created bug 1267144 and didn't see that there is already a bug for this.
Now I'm wondering whether this bug is meant for implementing the 'Break On XHR' button, i.e. break on any XHR, or for implementing the single XHR breakpoints, i.e. break on a specific XHR, or both.

Victor, can you please clarify that, so I know what to do with bug 1267144?

Sebastian
Flags: needinfo?(vporof)
See Also: → 1267144
Because Victor doesn't answer, Honza, can you please answer my question from comment 9?

Sebastian
Flags: needinfo?(vporof) → needinfo?(odvarko)
It isn't clear from the original report, but I think that this report should cover entire Break on XHR feature. The UI/UI requirements might change when somebody is actually working on this and so, any followups should be filed when we have more analysis.

@Sebastian, please close bug 1267144.

Honza
Flags: needinfo?(odvarko) → needinfo?(sebastianzartner)
Flags: needinfo?(sebastianzartner)
See Also: → dbg-dom-bps, 895893, 1165010
Assignee: nobody → zer0
Status: NEW → ASSIGNED
No longer blocks: important-firebug-gaps
Assignee: zer0 → nobody
Status: ASSIGNED → NEW
Target Milestone: --- → Future
Product: Firefox → DevTools
We've started a first-pass implementation of this, so assigning to myself to keep track of.
Assignee: nobody → lsmyth
Status: NEW → ASSIGNED
Attachment #9016647 - Flags: review?(dwalsh)
Attachment #9016647 - Flags: review?(dwalsh) → review+
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7f9ba7e8ac2
Update thread-client to work with xhr breakpoints
Keywords: checkin-needed
Attached patch 821610-xhr.patchSplinter Review
Attachment #9016647 - Attachment is obsolete: true
Attachment #9016729 - Attachment is obsolete: true
Attachment #9016739 - Flags: review?(lsmyth)
Attachment #9016739 - Flags: review?(lsmyth) → review+
Flags: needinfo?(lsmyth)
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffb770ed3871
Provide methods to set and remove XHR breakpoints within thread client r=loganfsmyth
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ffb770ed3871
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Where is the feature available on the UI? Thank you.
(In reply to istvan from comment #22)
> Where is the feature available on the UI? Thank you.

This bug report only fixes part of the backend support.

Related UI part is being fixed here:
https://github.com/devtools-html/debugger.html/pull/6934

I believe that the UI will appear in Nightly as soon as Bug 1501379 (new Debugger release) is resolved.

Honza
I noticed that #1501379 is marked as fixed for 65. Does that mean this feature won't show in the UI until 65?
Flags: needinfo?(odvarko)
(In reply to Irene Smith from comment #24)
> I noticed that #1501379 is marked as fixed for 65. Does that mean this
> feature won't show in the UI until 65?
Correct.

Honza
Flags: needinfo?(odvarko)
Attachment #9016729 - Attachment is obsolete: false

Created this page: https://developer.mozilla.org/en-US/docs/Tools/Debugger/Set_an_XHR_breakpoint

Added a mention of XHR breakpoints to the 65 Release notes

Added a link to the new page from the Debugger main page.

Please review the page and make sure that it is clear and complete.

Flags: needinfo?(lsmyth)

Thank you Irene for starting a draft!

Issues:

  1. Explain that the URL field is a subset match to the URL. You can user patterns like filter a path, file, querystring, etc; anything in the URL. Maybe an example like api/store/products/?
  2. Pause on any URL checkbox as wildcard option
  3. Link to XHR (specifically send, which triggers the pausing) and mention fetch, which is the modern XHR, which this panel also pauses on.
  4. The sentence When your code breaks on an XHR request, … and the parts after that are not correct. Those panes are always there when the Debugger is paused.

I second Harald's comments. I also just made one small change to the first sentence.

Flags: needinfo?(lsmyth) → needinfo?(irenesmith13)
Flags: needinfo?(irenesmith13)
You need to log in before you can comment on or make changes to this bug.