How One Missing `var` Ruined our Launch

Am I right in thinking that the javascript<p><pre><code> /* "use strict" */ </code></pre> construct would have caught this mistake (just like it would have done for me in perl code)? Seems to me that some such feature is absolutely and utterly necessary in any environment where you're doing a lot of closure creation ..

Reminds me of one of the more confusing bugs I've ever encountered in my life. I was throwing together a quick UI with Adobe Flex, and for some reason every time you clicked a particular button, the entire UI would shift 20 or so pixels to the right. I spent hours scratching my head until I noticed this for loop:<p><pre><code> for (x=0;x&#60;variable.length;x++) { // blah } </code></pre> I wasn't declaring the x variable, so it was using the x part of the x/y positioning of the UI container. Woops.<p>edit: initialise/declare brain freeze

&#62; I would posit here that nothing I could do in best practice (manual front-end testing, unit testing, error handling, etc.) would have caught the offending line.<p>jshint would have caught it. You need to run jshint on your code or you will get silly errors like this. Simple.

Since everyone is chiming in with ways to prevent this sort of thing, here is another: js2 mode for Emacs[1]. This is a mode originally written by Steve Yegge and then modified by some other people (be sure to get that version) that actually parses the code and, among other things, highlights global variables in a different color than local ones. I find this, along with the other things js2 does, helps prevent a whole host of annoying JavaScript issues, as I type.<p>[1]: <a href="https://github.com/mooz/js2-mode" rel="nofollow">https://github.com/mooz/js2-mode</a>

This is why I like both Scala and Coffeescript approaches. On Coffeescript vars are created for you, and on Scala, you can't not use it (you either use var or val, making it easy to change from re-assignable variables to final ones).<p>Note that nowadays going to Coffeescript from Javascript is quite easy: <a href="http://js2coffee.org/" rel="nofollow">http://js2coffee.org/</a>

One thing you can do to help avoid this: use JSLint (or something equivalent) to check for missing var keywords. And, the obvious (as you already mentioned) coffeescript. Would love to hear of other suggestions on how to effectively debug this, especially in node.

<i>I'm not an expert in JavaScript...</i><p>Given that you said that, and that the problem you hit is a common pitfall for Javascript developers (especially if you're not very seasoned with the language), I'd strongly recommend picking up a copy of Douglas Crockford's <i>Javascript: The Good Parts</i>. Not only does he inform readers of this particular gotcha, but he also elaborates on Javascript best practices and tools that others are bringing up in their comments.

Why has it become popular for dynamic languages to conflate establishing a binding with assigning it a new value? Ruby, Python, and Javascript are all guilty of this.<p>Scheme got it right sometime in the 1970's.<p><pre><code> (let ((x initial-value)) ; binding (set! x new-value)) ; assignment </code></pre> Or in infix syntax (Dylan):<p><pre><code> let x = initial-value ; x := new-value ;</code></pre>

<i>Ouch.</i><p>I always thought JS's global-variables-by-default schtick was it's worst crime, but I've never seen such terrible consequences for it up close before.<p>Note to self: before products get to forbes, do some load testing. Even if I am using node, with its magic event loop of invulnerability.

You will probably get hundreds of comments with unsolocited advice, but let me just say to checkout and setup JSLint:<p><a href="https://github.com/douglascrockford/JSLint" rel="nofollow">https://github.com/douglascrockford/JSLint</a><p>Setup and how you use it is more important than just using it. I run it in three places:<p>1) In my IDE (bound in vim on save, same in TextMate)<p>2) As a Git checking hook<p>3) In deployment / build scripts<p>Turn all the warnings way up and it always catches redeclaration bugs.<p>Set it up once and don't allow any code to get checked in or deployed with even a single warning present.<p>I also pass all Javascript through the Google Closure Compiler, but with the lowest compilation setting, because it is very good at picking up small errors as well.<p>(if anybody is interested in how to set this all up I can publish my configs and scripts)

This was a problem of laziness more than anything else. To the beginner developers out there: Learn to write good code and to pay attention to the details. Don't become just another co-founder trying to do the bare minimum just to make a buck. Take pride in your work and use best practices. Be careful to avoid the careless mistakes made by the author...<p><pre><code> 1. Did a major last minute code change 2. Did not run jslint 3. Did not use strict mode 4. Did not run load tests 5. Did not use an editor that catches scoping problems</code></pre>

JS is so easy to cock up in, I commonly find myself logging to the console just to make myself sure of the scope and other things.<p>This technique totally went to shit when I came across one of our scripts that gratuitously used `apply()` all over the place.<p>The other common error is array iteration, and I've not quite understood why iterating through one array in the same scope as where it was created works fine, but passing it to another function and performing the exact same routine also goes through the prototype methods after the elements.<p>Of course, particularly with the var mistake, you'd never really understand the magnitude of it until you attempted to use JS on the server side. This post has, quite thankfully, likely exposed a bug in my own code I couldn't quite understand a while ago. :)

Do I understand this right -- global variables are shared across all requests in node.js?<p>If so, that is an insane design.

"I would posit here that nothing I could do in best practice (manual front-end testing, unit testing, error handling, etc.) would have caught the offending line."<p>Running your code through JSLint (or something similar) would have been helpful here. Performing this check before committing code is considered a best practice and it's really easy to setup a pre-commit hook in most version control systems. JSLint has a Node.js mode. It would have said something like "variable used before being defined". <a href="http://www.jslint.com/" rel="nofollow">http://www.jslint.com/</a>

You can do some poor man's load testing very simply with siege (<a href="http://www.joedog.org/index/siege-home" rel="nofollow">http://www.joedog.org/index/siege-home</a>). Put a bunch of URLs in a text file, and run it with "siege -f urls.txt".<p>Of course it doesn't <i>prove</i> that your code is bug-free, but if you have concurrency issues, this will help shake them out. I've used this many times to reveal problems with DB connection pooling, concurrency issues, and excessive session size.

Melon Card.... I just signed up with a random email and it would allow me to remove the details for that email 'joe.doe@yourwebsite.com'... using your software as long as I did the captcha I could remove records for anyone's email as long as you can find them (the recommendation makes that easy)... you NEED to include verifying the email address before you can perform actions!!

Similar, but not quite as world-ending issue we had launching our Android app. Shortly (~36hrs) after launching, we had noticed that we were compiling with Cupcake (and we only intended to support Eclair+). We were pushing to have it out for SXSW; in a rush we just re-compiled, made sure it still launched, and pushed to the Market.<p>After everyone got some sleep, we realized that Cupcake didn't have multi-res support; and later phones quietly re-scaled everything to compensate. Compile to Eclair and well, things were a bit out of proportion.

I feel for you man, but did you not test with more than one user at the same time? I live in constant fear of this kind of thing, I always round up as many people as possible to test at once.

I think the main thing missing with Node is best practices. We use jslint with node. And we run tests in parallel, to flush out any dependencies. We had to wrap callbacks in a sequencing library to get good stack traces, though. Node.js also integrates with the Chrome V8 source level debugger, for real debugging!<p>Also, C++ can also take down your site with a single character typo that the compiler may not catch, so even static typing can't save against everything! (I actually like C++, too, so no hate there)

Is it just me or is this site completely unusable right now. I keep trying to say 'This isn't me!' or 'This is me!' with no results... maybe he has another missing 'var' somewhere?

Damn, I feel for ya; scope is hell sometimes, and js is all about giving you the power to shoot yourself in the foot. Interesting how bugs like this, while terrible and terribly easy to miss, tend to do a lot less damage on the front-end of things (or doesn't show up in a single thread/user environment). It was the potential of things like this happening that drove me towards languages like erlang and java on the server (even if js is still my favorite language).

<p><pre><code> &#62; (function () { internalvariablewithnovardelcaration = "string"; })(); &#62; internalvariablewithnovardeclaration ReferenceError: internalvariablewithnovardeclaration is not defined at [object Context]:1:1 at Interface.&#60;anonymous&#62; (repl.js:179:22) at Interface.emit (events.js:64:17) at Interface._onLine (readline.js:153:10) at Interface._line (readline.js:408:8) at Interface._ttyWrite (readline.js:585:14) at ReadStream.&#60;anonymous&#62; (readline.js:73:12) at ReadStream.emit (events.js:81:20) at ReadStream._emitKey (tty_posix.js:307:10) at ReadStream.onData (tty_posix.js:70:12) &#62; externalwithnovar = 1; 1 &#62; (function () { externalwithnovar = "string"; })(); &#62; externalwithnovar 'string' </code></pre> You're all wrong. There is no 'global by default for javascript. This guy clobbered a variable called initial outside of the scope of the callback.

Well whole rocket launches failed for silly reasons like temperature conversions, so you shouldn't feel too bad about it.

This is a major downside of interpreted languages, coupled with JS's diabolical scoping rules. Interpreted languages by their very nature, only reveal syntactic errors to you once you run them. I would like to see server side code written in compiled, statically typed languages, such as C/C++ to prevent such errors.

I hit a similar thing in some code I was writing, and ended up debugging it for about 5 hours. Couldn't figure out why when some tests ran, one of them would just die.<p>Turns out I forgot to use 'var' in a library function, and the tests were running concurrently, and so one clobbered the other. It was not fun.<p>Been using JSHint ever since.

A un-initialised varible made our script to mail 20 copies of the same email to almost 10k users. One of our mass email script basically breaks the email list in chunks of 500 and emails them. The array variable that hold this 500 email chunk was not initialised. In our test everything was fine as the test list was never more than 500. When the time came to send emails to the production list, the script basically kept sending multiple copies of the same emails (since the array retained the old values across the loop iterations).<p>I realised this after about 10 minutes but already multiple copies of the same emails (20/user) was send to more than 10k users!...The end result was annoyed users unsubscribing in droves and possibly a hit in our email reputation. This is all because a stupid array variable was not initialised :)

Screams lack of testing. Speaking of which, did you know your site design breaks when viewed on an iPad?

Use jslint, Luke.

Common problem. Live and learn.<p>But the problem is a bit more meta than Javascript scoping. Something would have happened no matter your environment. It's always extremely risky to make huge changes just before a launch day. Stuff always breaks. Sounds like you did a good job of communicating with your customers. Ironically, they will probably be better customers than they would have been if you didn't have any problems. Overcoming adversity brings people together.<p>Add some concurrency to your test suite. If you have any system level or black box tests, you can often just run the same test multiple times in parallel with different threads. No new tests required. Note that this may (probably will) also uncover other previously inconceivable concurrency issues.

As many have pointed out, there are absolutely tools that would catch this kind of problem in development. But the experience also speaks to the lack of sophisticated observability tools for Node (and just about every other popular dynamic language too).

This is a lesson we all have to learn. At least in Node it is easy to pick up with a tool like JSLint. In Java, you typically have some classes which have to be thread-safe (having no per-request state) and others which are instantiated per request. Developers have to be aware of the distinction, and to code accordingly. I fell into this trap when submitting a change to JBoss back in 2000; Rickard Oberg picked it up within hours (the diffs of all committed changes went out to everyone on the developer mailing list). I'm grateful to him for that, and I've done the same kind of review for many others since then.

This is why I really dislike it when I see<p><pre><code> var a = "foo", b = "bar", c = "baz"; </code></pre> All it takes is one missed comma and all of a sudden you've turned a <i>lot</i> of your variables global.

And that is what happens when you throw static type checking and the other safety features of compiled languages out the window, 'cause all the cool kids are doing it.

Maybe a language where variables are global by default and there are no concurrency semantics at all isn't so great for serving up web apps to concurrent users?

This reminds me of something I could not do. How do you load test a nodejs - socket.io installation? Is there a tool like apache bench to load test websockets?

I don't see this as a JavaScript specific problem. A similar issue can occur in IIS, for example. The details would be more elaborate, but the core issue would remain the same. A variable shared across requests screws something up under load. Seen it happen.<p>This is one of the reasons I like shared-nothing architecture where the only way to share or persist something is via explicit caching or database.

Thanks for reminding me why I like Haskell so much.

My var story was a little harder scoping issue where a variable held the information for a pending call. It would asynchronously process the call, then mark the call as completed. Well, if there were more than one call going through, the callback was only getting one record, causing it to repeat the call about five times before I caught it and fixed the scoping.

Very interesting. I've been thinking about delving into Node.js for a while, and I will definitely keep things like this in mind.

Do JSLint or JSHint catch the common error of using "this" inside a closure when you want it to be lexically bound instead of dynamically?<p>It's easy to forget to go "var me = this;" and use me instead of this inside of closures. But how would JSLint or JSHint know you made a mistake and didn't actually mean for "this" to be bound dynamically?

I know there seem to be a few HN readers religiously against using an IDE, but in this case an IDE would have probably helped. Since it colors global and local variables differently the error would be much more apparent at the time when it was made (you typically have very few global variables so they stand out color wise).

Steve Yeggae's js2-mode highlights globals during editing, and it's saved me a good many times.<p><a href="http://code.google.com/p/js2-mode/" rel="nofollow">http://code.google.com/p/js2-mode/</a><p>Mind you it won't catch globals declared in chained assignments<p>eg<p><pre><code> var x = y = 0; // y is defined globally</code></pre>

I don't always write JavaScript (CoffeeScript FTW) but when I do I use JSLint (vim-jslint)...

Congrats on launching. Saying this ruined your launch is a bit dramatic. The ability to put out fires when they happened is crucial to startups. Seems like the problem only existed for a few hours. It could have been worse. Write a test and move on

Oh wow. I've faced this exact same problem with missing `var`s more than once before. It's pretty ridiculous that JavaScript makes variables global by default.<p>Sorry to hear about your tragedy, hope it doesn't cause too many issues in the long run.

Reminds me of a time i had a similar bug.The more i tried the more i found it hard to discover the bug.One approach i usually use is to take a timeout and relax.But in your position there was no time for relaxing.

Node definitely needs better error handling, but what's nice about node is that each module is scoped in its own namespace, so if he had put each of his routes in its own module, that wouldn't have happened.

Ack, stuff likes this makes me very nervous about our production nodejs backend ... the error tracking is really quite difficult. But it's more a byproduct of javascript as opposed to node/express.

(Shameless plug, but interesting enough, imho). This is why I created scopeleaks: <a href="https://github.com/ruidlopes/scopeleaks" rel="nofollow">https://github.com/ruidlopes/scopeleaks</a>

The real failure that opened you up to this race condition is naming. Initial is not specific enough and thus colided with another variable, that was also not named specifically enough.

&#62; I would posit here that nothing I could do in best practice would have caught the offending line.<p>bullshit.<p>jslint or clojure compiler would have found this error at "compile time."<p>coffeescript would have made it a non-issue.

"It’s beautiful simple..."<p>I might just be painfully slow, but the implementation he described didn't sound anything remotely simple to me (though I believe it really is beautiful).

Wow, thanks for sharing!<p>It's these small things to remember that can save a lot of debugging time down the road. Will remember to "use strict" when I use nodeJS again.

Exactly why we moved to CoffeeScript, without it you need to rigorously enforce coding conventions. This entails watching for missing vars, using jslint, having a script that searches for accidentally declared globals, only declaring vars at the top of a function, among other best practices. With CS you get all of that just by using it. If you can write high-standard code using a simpler, terser syntax, why avoid it?

I just hit your page and it gave me an error that said the server was over capacity. Not the only reason?

jslint to the rescue. If all your source is JS, there's no excuse to at least use lint.

Php shares nothing except session between requests and local scope is default.

&#62; I would posit here that nothing I could do in best practice (manual front-end testing, unit testing, error handling, etc.) would have caught the offending line.<p>Sorry, that's incorrect. If you cannot fully simulate your environment for purposes of validation, you're not covering your bases.

Always use JSLINT.

for (var key in this) console.log(key);

jslint

"It’s a damn tragedy.  I’m not an expert in JavaScript"<p>Hell yea, the valley's full of smart kids who don't know what the hell they're doing with JavaScript and thinks they're one hell of a hacker when they discovered that they can do shit with a few lines of JS and trumpeting on the how great NodeJS or whatever the greatest and newest shiny framework out there.<p>If you don't really understand scoping in JS, please don't use node and hit yourself on your foot and then blog the cool shit out of it.<p>I'm just saying, no hard feelings. :)

Crazy. Same thing happened to me. Good luck with the next project.

"Now is the point I’d like you to say: you should have used CoffeeScript, and hey TameJS while you’re at it. You’d be right."<p>Perfect example of a developer hating JS just because he/she didn't bother to learn it first and got burned.

Actually it is very easy to debug any javascript error. In IE there is a setting where you can uncheck the Disable Script debugging (Internet Explorer) and in the status bar you will see any javascript error. If you check the checkbox IE will not report any errors. In general during your development you should always uncheck the box.If you double click the javascript status this will exactly report the line number where the javscript error is. I exactly don't know if you see these kind of errors in Chrome and firefox. I know people blame IE but there are quite few nice handy features which are very helpful during development