1644186 - Implement JS Error listening via the ResourceWatcher API on the actor side

Status: RESOLVED FIXED

(DevTools :: Console, task, P1)

Description

[Alexandre Poirot [:ochameau]]

The goal here is to implement the ERROR_MESSAGE resource from the actor side.
This is about replicating the current behavior, implemented in bug 1625905.
But instead of having a wrapper on the client side to morph the legacy WebConsoleActor methods (i.e. the legacy listener code),
we would implement an actor API matching ResourceWatcher API (i.e. watch and unwatch).

This would be about implementing a server side equivalent of this legacy listener code:
https://searchfox.org/mozilla-central/source/devtools/shared/resources/legacy-listeners/error-messages.js
In a new server side module:
https://searchfox.org/mozilla-central/source/devtools/server/actors/resources/error-messages.js

This module would typicaly look like this:

const { TYPES } = require("devtools/server/actors/resources/index");

class MyResourceWatcher {
  /**
   * Start watching for all ${MY_RESOURCE_TYPE} related to a given Target Actor.
   * This will notify about existing ${MY_RESOURCE_TYPE}, but also the one created in future.
   *
   * @param TargetActor targetActor
   *        The target actor from which we should observe console messages
   * @param Object options
   *        Dictionary object with following attributes:
   *        - onAvailable: mandatory function
   *          This will be called for each resource.
   */
  constructor(targetActor, { onAvailable }) {
    // In most cases, we already have some helper class which helps observing one resource
    // that we can spawn like this:
    // Note that it may often be easier to merge such `MyResourceListener` into this `MyResourceWatcher` class!
    const listener = new MyResourceListener(
      targetActor.browsingContextID,
      targetActor.window,
      ...  /* whatever is useful for your observation */
    );
    
    // Forward all future resources being observed to the upper layer calling this module,
    // via `onAvailable` callback argument.
    // I'm using EventEmitter API here, but the API may different,
    // based on the platform API we have to use to observe the resource.
    listener.on("one-of-my-resource-is-created", resource => {
      // We have to ensure that each resource object has a valid `resourceType` attribute
      resource.resourceType = TYPES.MY_RESOURCE_TYPE;
      onAvailable([resource]);
    });
    
    // Also forward all resources which already exist when we are calling this method
    // (if any exists)
    const cachedResources = listener.getAllAlreadyExistingOrCachedResources();
    for(const resource of cachedResources) {
      resource.resourceType = TYPES.MY_RESOURCE_TYPE;
    }
    onAvailable(cachedResources);
    
    // Save the listener in order to destroy/stop watching later on.
    this.listener = listener;
  }

  /**
   * Stop watching for ${MY_RESOURCE_TYPE}.
   */
  destroy() {
    if (this.listener) {
      this.listener.destroy();
    }
  }
}
module.exports = MyResourceWatcher;

An important goal here is to emit the exact same resource object that the legacy listener is passing to its onAvailable callback.
Same attributes, same values, ...

Bug 1644185 could be used as a template. As it did this work for PLATFORM_MESSAGE resource type.

The main reason to do this is to be able to start listening to the resource before the page starts loading.
Thanks to the framework work done in bug 1620243, this MyResourceWatcher class will be instantiated before the page starts
loading and possibly as early as the content process just started.
This wasn't the case with legacy actor APIs like WebConsoleActor.startListeners, ThreadActor.attachThread, ...
We were calling these methods too late, only after the frontend is notify about the existance of the target, so, late after the page started loading.

You will also have to register this new module in this registry:
https://searchfox.org/mozilla-central/source/devtools/server/actors/resources/index.js

• Add a new entry in TYPES object.
• Register your new resource watcher module into Resources object.

Last but not least, it is probably a good time to review the existing tests for this Resource:
https://searchfox.org/mozilla-central/source/devtools/shared/resources/tests
And ensure that it has a good coverage.
You would especially have to migrate all Client/Front tests, which were testing the backend behavior via targetFront.getFront("myfront").
All these tests will be removed, once we drop the legacy listeners. Because we are going to drop the server API that we no longer use.
Like WebConsoleActor/Front.getCachedMessage(), WebConsoleActor/Front.startListeners(), ThreadActor/Front.sources(), ThreadActor/Front.new-source, ...

Comment 1

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m2 bugs for Fission Nightly (M6c)

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Let's wait for Bug 1625910 to land before tackling that, so js errors and css warnings are distinct

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1644186 - Add server support for Error messages resources. r=ochameau.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1644186 - Move helper functions to helper files. r=ochameau.

Functions to creates object actors were duplicated between error-messages
and console-messages watchers, so we put them in helper files instead.

Depends on D81356

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1644186 - Add ResourceTransformers. r=ochameau.

Depends on D81490

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19bc3a5c0122
Move helper functions to helper files. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/6c15271350bd
Add ResourceTransformers. r=ochameau.

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/19bc3a5c0122
https://hg.mozilla.org/mozilla-central/rev/6c15271350bd

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/162bc74a2c75
Add server support for Error messages resources. r=ochameau.

Comment 9

[Razvan Maries]

Backed out for perma failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-devtools-chrome-e10s-11%2Cm%28dt11%29&revision=162bc74a2c75c03ab53674f5f118ae4f3e7afa34&selectedTaskRun=Sg5vaf80Q8iUbHIu1Jp5Yg.0

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309124035&repo=autoland&lineNumber=11165

Backout: https://hg.mozilla.org/integration/autoland/rev/4b6aac3c7b427cf85ad574329a62d23270222d33

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

ah, sorry, forgot to update something when I disabled the trait for error resources. I updated the patch and the test is fine now

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aebce691bd57
Add server support for Error messages resources. r=ochameau.

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/aebce691bd57