1849920 - Add support network response override for any request in the netmonitor

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement)

Description

[Hubert Boma Manilla (:bomsy)]

Currently we can't local override html files as this is disabled for those.

This is because the current approach for local overrides crashes for html files.

Linking to comment which might be useful when fixing https://phabricator.services.mozilla.com/D163600#anchor-inline-931473

Comment 1

[Julian Descottes [:jdescottes]]

Note that Channel's setResponseOverride added in Bug 1900375 should allow you to override any resource.
The downside at the moment is that the netmonitor will fail to get the response updates for the request.
Although we might be able to fix that when the override is created from devtools.

Comment 2

[Julian Descottes [:jdescottes]]

Note that in some cases (eg https://stuff.overengineer.dev/mozilla/testcases/devtools-script-override-sri/) the HTML does not contain any script, so we can't override it from the debugger because we can't see it in the source tree.

Comment 3

[Julian Descottes [:jdescottes]]

Trying to consider what a minimal effort solution could be done here, I would propose to:

• add a resource (or events) to monitor overridden URLs
• update the debugger frontend to use this resource to maintain mutableOverrideSources
• add a isOverridden flag on network events
• update netmonitor UI to show network requests with isOverridden slightly differently (eg same dot icon as in the debugger + maybe say "overridden" in the transferred column)
• add a context menu item in the network monitor to set and clear an override
• finally, lift the restriction in the debugger UI to only set an override on non-HTML sources

Alex, how does that sound to you as a first step here to unlock the feature?

Comment 4

[Alexandre Poirot [:ochameau]]

I'm only wondering about the two first bullet points. The word "resource" sounds weird as resources are typically created and managed by the server, whereas here it sounds like a frontend-only thing.

This rather reminds me the pattern used for DOM Mutation Breakpoints, where we need to share them between Inspector and Debugger.
For that we have a "toolbox" store in order to share the dom-mutation-breakpoints reducer between many panels:
https://searchfox.org/mozilla-central/source/devtools/client/framework/reducers/dom-mutation-breakpoints.js
This data is being used by the frontend over these:
https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/components/SecondaryPanes/DOMMutationBreakpoints.js#146
https://searchfox.org/mozilla-central/source/devtools/client/inspector/markup/markup.js#435
(this really should be used selectors so that we easily know where this is being used...)

I imagine we could follow the same technique in order to share the currently overriden sources in React codebases of Debugger and Netmonitor.

The debugger already registers the two distinct stores:
https://searchfox.org/mozilla-central/rev/de46cd99a7d1634058b511a3f546a970763e5048/devtools/client/debugger/src/utils/bootstrap.js#91-97
(The inspector does weird things as it's not yet full react)
I imagine we would have to tweak that over there for making it word for the Netmonitor:
https://searchfox.org/mozilla-central/rev/de46cd99a7d1634058b511a3f546a970763e5048/devtools/client/netmonitor/src/app.js#76

Otherwise the overall plan sounds good to me. As soon we are confident that we can't set an override which would cause the request to disappear from the netmonitor, it all sounds good. Otherwise, if we aren't, we should prioritize having a panel showing the overrides.

Comment 5

[Julian Descottes [:jdescottes]]

(In reply to Alexandre Poirot [:ochameau] from comment #4)

I'm only wondering about the two first bullet points. The word "resource" sounds weird as resources are typically created and managed by the server, whereas here it sounds like a frontend-only thing.

Ah interesting, it felt really more natural for me to design it as a server-owned resource.

In practice the overrides are owned by the server:

• the source of truth for the list of overrides is the NetworkObserver's #overrides map
• the command to set / remove an override routes it to the network front: https://searchfox.org/mozilla-central/rev/de46cd99a7d1634058b511a3f546a970763e5048/devtools/client/debugger/src/client/firefox/commands.js#448-465, and then the corresponding actor calls the necessary API on its NetworkObserver

But I guess it's the same for DOM Mutation breakpoints, they must be at some point sent to the server and persisted there. So in this context "frontend-only" probably means "something that can only be set / updated by the devtools frontend", which is true at the moment.

The thing I like with the resource approach here, is that we make it clear that the source of truth for the information is the server. The frontend can consume it as it wants via watchResources, and update it via commands. There is no dependency on react/reducers/actions/etc, just watcher + commands, which feels like a simpler architecture. I think it's also more future proof. If we ever want to share the overrides between toolboxes for instance.

I'm OK with skipping the resource bit and instead having a shared reducer for this, given that it's similar to mutation breakpoints.

I would still be curious to discuss about using resources for things which are frontend driven but stored on the server. It still sounds very compelling to me to use the commands + resource pattern to handle them consistently with the rest of the codebase.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Update network overrides from the netmonitor

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Move the network overrides to a shared toolbox reducer

Depends on D219229

Comment 8

[Phabricator Automation]

Comment on attachment 9419235 [details]
Bug 1849920 - [devtools] Move the network overrides to a shared toolbox reducer

Revision D219235 was moved to bug 1914657. Setting attachment 9419235 [details] to obsolete.

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Add Override column in netmonitor

Depends on D219220

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Update file and URL columns in netmonitor to show override path

Depends on D232349

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Show the override icon in the netmonitor response panel

Depends on D232349

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Automatically show and hide the override column in the netmonitor

Depends on D232350

Comment 13

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Fix memoization issues for netmonitor getColumns selector

The individual input selectors of reselect selectors should be as close as possible to the
data used by the selector, so that we can efficiently memoize the selector.
Since this selector returns an object which will always fail shallow comparisons it's important to
make sure we memoize efficiently.

Comment 14

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Update netmonitor requests selectors to improve memoization

Depends on D234517

Update input selectors to memoize selector functions more efficiently.

Comment 15

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Avoid unnecessary renders due to blockedUrls prop

Depends on D234519

Comment 16

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Improve request selectors to avoid rebuilding filters for each request

Depends on D234519

Comment 17

[Phabricator Automation]

Comment on attachment 9459928 [details]
Bug 1849920 - [devtools] Fix memoization issues for netmonitor getColumns selector

Revision D234517 was moved to bug 1943339. Setting attachment 9459928 [details] to obsolete.

Comment 18

[Phabricator Automation]

Comment on attachment 9459931 [details]
Bug 1849920 - [devtools] Update netmonitor requests selectors to improve memoization

Revision D234519 was moved to bug 1943339. Setting attachment 9459931 [details] to obsolete.

Comment 19

[Phabricator Automation]

Comment on attachment 9460238 [details]
Bug 1849920 - [devtools] Improve request selectors to avoid rebuilding filters for each request

Revision D234704 was moved to bug 1943339. Setting attachment 9460238 [details] to obsolete.

Comment 20

[Phabricator Automation]

Comment on attachment 9459934 [details]
Bug 1849920 - [devtools] Avoid unnecessary renders due to blockedUrls prop

Revision D234521 was moved to bug 1943339. Setting attachment 9459934 [details] to obsolete.

Comment 21

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Pass the toolbox store from console to TabBox

Depends on D233302

Comment 22

[Julian Descottes [:jdescottes]]

Attachment: Bug 1849920 - [devtools] Add browser mochitests for network overrides

Depends on D235540

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d76ca7f0f8b9
[devtools] Update network overrides from the netmonitor r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/00353ba2d76d
[devtools] Add Override column in netmonitor r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/5cf978e01585
[devtools] Show the override icon in the netmonitor response panel r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/ffc748fee434
[devtools] Update file and URL columns in netmonitor to show override path r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fa63c9600a64
[devtools] Automatically show and hide the override column in the netmonitor r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/bcec102004cd
[devtools] Pass the toolbox store from console to TabBox r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/e9742b2e36d9
[devtools] Add browser mochitests for network overrides r=devtools-reviewers,nchevobbe

Comment 24

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d76ca7f0f8b9
https://hg.mozilla.org/mozilla-central/rev/00353ba2d76d
https://hg.mozilla.org/mozilla-central/rev/5cf978e01585
https://hg.mozilla.org/mozilla-central/rev/ffc748fee434
https://hg.mozilla.org/mozilla-central/rev/fa63c9600a64
https://hg.mozilla.org/mozilla-central/rev/bcec102004cd
https://hg.mozilla.org/mozilla-central/rev/e9742b2e36d9

Comment 26

[Sebastian Zartner [:sebo]]

As far as I can see, this change not just allows local overrides for HTML files but also adds a UI to the Netmonitor for all kinds of overrides. And this new feature is not behind a pref. Julian, is that correct?

If so, I think this should be mentioned in the developer release notes as well as the Firefox release notes for 137.

Sebastian

Comment 27

[Julian Descottes [:jdescottes]]

Yes to all questions. And good point, we should definitely add it to the release notes. FWIW I just added user docs via Bug 1948304 and I'm planning a blog post on fxdx.dev to publish in time for 137 release.

Comment 28

[Sebastian Zartner [:sebo]]

Fantastic! So let's add the required flags and comments, so it's not missed.

Release Note Request (optional, but appreciated)
[Why is this notable]: Allows local overrides from within the Netmonitor.
[Affects Firefox for Android]: no
[Suggested wording]: The Netmonitor now allows to override network request responses with local files.
[Links (documentation, blog post, etc)]: TBD, see Julian's comment above.

Sebastian

Comment 29

[Pascal Chevrel:pascalc]

Julian, do you mean the Network panel in Developer tools when you say Netmonitor? Should there be a screenshot of the feature? Thanks

Comment 30

[Julian Descottes [:jdescottes]]

(In reply to Pascal Chevrel:pascalc from comment #29)

Julian, do you mean the Network panel in Developer tools when you say Netmonitor? Should there be a screenshot of the feature? Thanks

Yes sorry I meant the Network panel (we call it netmonitor in the codebase).

There are a few screenshots at https://firefox-source-docs.mozilla.org/devtools-user/network_monitor/network_overrides/index.html which we could reuse?
Maybe the first one.

Comment 31

[Pascal Chevrel:pascalc]

Added with a screenshot and this wording:

The Network panel now allows overriding network request responses with local files.

I can add a link to the blog post when available, just ping me in this bug.
I am keeping the relnote flag open until we ship the feature to the release channel

Thanks!