Our team has recently been cracking down on the JavaScript errors that have been occuring in our application. Whenever a JS error occurs, we make sure it gets logged into our logging system. Then every week we take a look at those errors and try to figure out why they occurred and what we can do to fix them.
The Problem
I was tasked to take a look at one particular error not too long ago. The error message being reported was a classic:
Uncaught TypeError: x.split is not a function
Though this was an error from production running minified code, there was enough information for me to figure out where the issue was coming from. It was happening in a subclass of a Backbone Router. This subclass expected a string argument that in the initialize method (effectively the constructor in Backbone) was split into other, individual string parts. It went something like:
var RouterSubclass = Backbone.Router.extend({
initialize: function ( whole ) {
var parts = whole.split( regExp );
// ...
}
// ...
The way it was created looked something like:
var router = new RouterSubclass( params.whole || '' );
Here, params is an object containing collection of string-based parameter values. The whole parameter is passed into the RouterSubclass constructor which then ultimately calls the initialize() seen defined above. If whole doesn’t exist, it makes sure an empty string is passed in instead of undefined, though we pretty much always expect whole to exist.
The Investigation
Upon initial inspection, there didn’t seem to be anything especially wrong with this code. The most obvious offender would have been params.whole. If this were not a string value, then split() would not be found and we’d get the error seen in our logs. But there was nothing in the code that could have possibly made whole anything other than a string. And if it didn’t exist at all, the value passed into the constructor would have been an empty string which would have also been usable with split().
Still, it was obvious something was wrong. While we’ve never experienced this error internally on our own dev servers, somehow people out in the wild were hitting it. Not many, but enough to be noticeable.
While I don’t work with Backbone directly much any more (this was legacy code), I did remember that Backbone objects tended to prefer accepting arguments as objects. Views, for example, expected a single options argument:
new View({ ...options })
while Model objects accepted both an attributes object as well as an options object:
new Model({ ...attributes }, { ...options })
The same was the case for Router objects. When creating a Router, it expected, much like Views, a single options object:
new Router({ ...options })
What was interesting about our RouterSubclass was that it did not accept an options object. Rather, it accepted a single string value. Though the Router base class wanted an object, our RouterSubclass seemed to be working fine using a string instead. It wasn’t following the precedent set forth by Router, yet it still somehow worked. I didn’t like that it worked, and had a suspicion that maybe that was part of the problem.
Luckily, Backbone publishes an annotated version of their source code making it really easy to see what’s going on in their code with nice little descriptions that provide additional detail. I looked up the Router class and found the constructor:
var Router = Backbone.Router = function(options) {
options || (options = {});
this.preinitialize.apply(this, arguments);
if (options.routes) this.routes = options.routes;
this._bindRoutes();
this.initialize.apply(this, arguments);
};
It’s not especially complicated. The constructor is mostly just a wrapper for initialize() (and a preinitialize()) along with a couple of other bookkeeping items. The arguments from the constructor are simply forwarded right along to initialize(). But then I saw it - the reason for the failure. Do you see it too? (If you’d like a hint, see this JS Tip of the Day.)
The Cause
The cause of the problem is subtle. It’s also a thing I’d never thought would have any real-world consequences, and yet here it was, in production code, by means of a (once) popular frontend framework. It all had to do with changing signature of the subclass from an object to a string.
It turns out that parameters in JavaScript can be reference variables. It’s the only place in JavaScript variables like this can exist, and it only occurs when not using any new parameter list features like default or rest parameters. What this means is, if you change the value of a parameter variable, it can also change the respective value of that parameter in the arguments object and vice versa. In the case of Router, this means when it assigns a default options object with:
options || (options = {});
Its also changing that value in the arguments object - the same object that’s being used to forward arguments to initialize() via:
this.initialize.apply(this, arguments);
In the case of our code with our string-accepting subclass, if we pass whole as an empty string, which is it’s default value when params.whole doesn’t exist, that empty string as options inside Router, being falsy, is converted from a string into an empty object. Because options is a reference to that same value in arguments, the argument that makes it into initialize() as whole is then also an empty object. When RouterSubclass's initialize() tries to call split() from this empty object, we get the error seen from the logs.
var parts = whole.split( regExp );
// expected whole='', became whole={}, error thrown
The root cause goes even deeper (involving timing and validation within initialization chains), but we can save that story for another day.
The Solution
The solution was a simple one. It basically meant updating RouterSubclass to use an object argument instead of a string. The whole value would then be a property of that options object. Inside the constructor a check was also added to make sure it existed and was, infact, a string.
initialize: function ( options ) {
if ( typeof options.whole !== 'string' ) {
throw new Error( 'Required RouterSubclass option "whole" is invalid or does not exist.' );
}
var parts = options.whole.split( regExp );
// ...
}
When I wrote the JS Tip of the Day covering this behavior, I wondered if it would really be useful to anyone, ever, or would it just be useless knowledge taking up brain space? Little did I know it would be something I’d run into myself.