Labs/Jetpack/So You're Implementing a JEP

From MozillaWiki
< Labs‎ | Jetpack
Revision as of 03:53, 8 May 2010 by Adw (talk | contribs) (→‎try-catch-log: Corrected this section, I think)
Jump to navigation Jump to search

So you're implementing a JEP.

Most of this applies to the high-level APIs only, the ones covered by JEPs. Low-level modules, which have mostly been written by Atul, follow different conventions.

Landing your Implementation

The process from JEP to landing has worked something like this so far:

  • JEP first draft on wikimo.
  • Ask Myk for review.
  • He'll start a conversation in the user group so everybody can offer feedback.
  • Iterate on the JEP, updating the wiki, based on the conversation.
  • Final JEP draft agreed upon.
  • Ask Myk to review your patch. He'll check that it more or less implements the JEP.
  • Ask Atul to review your patch. He'll check that it uses the Jetpack platform OK.
  • Ask a domain expert to review your patch.

Modules

The module you're writing is a CommonJS module and is similar to a Mozilla JavaScript module (.jsm).

All the properties you attach to the exports object are visible to the outside world. Everything else is private.

Loading

Instances of your module are created by the Cuddlefish loader, which is part of the Jetpack platform.

A new instance is created the first time an extension require()s it, but thereafter that instance is generally cached for that extension. However, the loader is capable of creating multiple concurrent instances of your module should the need arise, so you shouldn't write your module such that it depends on there being only one instance of it at a time. For each instance, the loader creates a Cu.Sandbox and evaluates your module's code inside it.

Unloading

Your module can register a callback that will be invoked when the module receives an "unload message". When your module is used in an extension, this message will be sent when the extension is disabled or uninstalled. In general, though, the message can be sent at any time. Unit tests, for example, often cause it to be sent to test that all resources owned by a module are freed.

Your module should register an unload callback if it owns resources or causes side effects that should not persist beyond its "lifetime". For example, XHR module registers an unload callback that aborts all pending requests. The context menu module registers an unload callback that removes all the elements it's added to the DOMs of chrome windows.

Registering an unload callback is easy. The unload module provides two helper functions: when() and ensure(). when() takes a callback. ensure() takes an object that defines an unload() method. When you call require("unload").ensure(obj), obj.unload() is called once and only once. All calls after the first become no-ops. If you call it before the unload message is sent, it will not be called again; if you call it twice, the second call will be a no-op; if you don't call it, it will be called when the unload message is sent.

When do you use when() and when do you use ensure()? Only use when() in the global scope of your module. That's because the unload module keeps a reference to your when() callback -- and therefore anything in the scope of that callback -- for the lifetime of the extension. You can always use ensure().

Note that all (proper) modules already clean up after themselves, and if the only cleanup your module needs to do is related to the resources of other modules, you might not even need to do anything at all.

If however your module uses an unload callback, your unit test should ensure that it works as expected. See the XHR test for an example of forcing the unload message to be sent.

Private Parts

You make "private" members in Mozilla JavaScript code by prefixing their names with an underscore (e.g., this._dontTouchMe), but that's no good for Jetpack, where we're trying to enforce, you know, a security model.

Whenever you define a property on an object, stop and think if that object is exported -- if it's public. Yes? Is that property "private"? Yes? Then it's not really private at all.

Also, clients can apply your public methods to any arbitrary object by using call() and apply(). You therefore can't be sure that this really refers to your object. It might be an object that does something untoward when applied to the code you've written for yours.

For these reasons, define all public methods on this inside your class's constructor, and instead of using this inside those methods, use variables in the scope of the constructor.

// BAD
function MyExportedObject(someProperty) {
  this._someProperty = someProperty;
};
MyExportedObject.prototype = {
  publicMethod: function () {
    return this.alsoPublic();
  },
  alsoPublic: function () {
    return this._privateHelper();
  },
  _privateHelper: function () {
    return this._someProperty;
  }
};

// GOOD
function MyExportedObject(someProperty) {
  const self = this;

  this.publicMethod = function () {
    return self.alsoPublic();
  };

  this.alsoPublic = function () {
    return privateHelper();
  };

  function privateHelper() {
    return someProperty;
  }
};

Client Callbacks

try-catch-log

Always try-catch callbacks passed into your API and log the error with console.exception(). Two reasons:

  1. You don't want user exceptions interrupting control of your own code.
  2. Exceptions thrown by anything that's ultimately called by Jetpack should not propagate back into the platform. Because if they do, they get jumbled in with all the other errors coming in from web pages, browser chrome, etc., and their tracebacks don't get logged. console.exception() should always be at the top of the call stack.

There is an errors module that currently provides some related helpers and could stand to be expanded. catchAndLog() returns a function that wraps a callback in a try-catch-log.

// BAD
someObj.callback();

// GOOD
try {
  someObj.callback();
}
catch (err) {
  console.exception(err);
}

this

Think about what object this should refer to inside your client's callback. (It might be obvious, but if not, it should be specified in the JEP.) In particular, be careful about calling a callback that isn't attached to an object. If you call it as you normally would a function, this is the current global context. What's the global context inside your module code? It's not your module, by which I mean the exports object. It's the Cu.Sandbox that Cuddlefish used to evaluate your code! Not something that your clients should be able to access.

// BAD
callback();

// BETTER
callback.call(exports);

// BEST
someRelevantObject.callback();
callback.call(someRelevantObject);

Exceptions

Never throw raw strings, because console.exception(), which is ultimately used to log any exceptions in Jetpack, can't introspect a string to get at any additional information about where the error came from. Instead, throw a new Error(), which automatically adds metadata about the state of the stack at the time the error was thrown. This allows us to provide helpful tracebacks to developers.

Being friendly to developers is an important part of Jetpack. Therefore, all error messages that developers might see should be informative and punctuated full sentences.

XPCOM exceptions generally aren't friendly, so they should be caught and friendly errors thrown in their place -- within reason of course. You don't need to catch every possible XPCOM exception that might be thrown by a line.

// BAD
let stream = Cc['@mozilla.org/network/file-output-stream;1'].
             createInstance(Ci.nsIFileOutputStream);
stream.init(file, openFlags, permFlags, 0);

// GOOD
let stream = Cc['@mozilla.org/network/file-output-stream;1'].
             createInstance(Ci.nsIFileOutputStream);
try {
  stream.init(file, openFlags, permFlags, 0);
}
catch (err if err.result === Cr.NS_ERROR_FILE_NOT_FOUND) {
  throw new Error("The file " + fileName + " does not exist.");
}

Testing

Your module should come with unit tests.

If you use the unload module to clean up resources you've created, you should test that those resources are actually cleaned up when your module receives the unload message. See #Unloading above for details.

Modularity

Might parts of your code be useful to others? Break them out into their own bugs so others can benefit.

See Also