# Broken Router Post-mortem **URL:** \ **Category:** web dev\ **Created:** [August 24, 2020, 3:18pm UTC](https://forum.kirupa.com/t/broken-router-post-mortem/643960 "2020-08-24T15:18:56Z")\ **Posts on this page:** 1\ **Page:** 1
**Author:** ![senocular](https://yyz1.discourse-cdn.com/flex011/user_avatar/forum.kirupa.com/senocular/32/7217_2.png) [@senocular](https://forum.kirupa.com/u/senocular)\ **Post date:** [August 24, 2020, 3:18pm UTC](https://forum.kirupa.com/t/broken-router-post-mortem/643960/1 "2020-08-24T15:18:56Z")
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: ```nohighlight 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](https://backbonejs.org/) 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: ```javascript var RouterSubclass = Backbone.Router.extend({ initialize: function ( whole ) { var parts = whole.split( regExp ); // ... } // ... ``` The way it was created looked something like: ```javascript 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: ```javascript new View({ ...options }) ``` while Model objects accepted both an attributes object as well as an options object: ```javascript 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: ```javascript 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](https://backbonejs.org/docs/backbone.html) 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](https://backbonejs.org/docs/backbone.html#section-205) class and found the constructor: ```javascript 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](http://forum.kirupa.com/t/643093).) ## 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: ```javascript options || (options = {}); ``` Its also changing that value in the `arguments` object - the same object that’s being used to forward arguments to `initialize()` via: ```javascript 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. ```javascript 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. ```javascript 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](http://forum.kirupa.com/t/643093) 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.