222 comments

[ 4.1 ms ] story [ 203 ms ] thread
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).

Note that nowadays going to Coffeescript from Javascript is quite easy: http://js2coffee.org/

I don't know Coffeescript - does it implicitly create a local var when there is already a global with the same name?
CoffeeScript is a compiler, so obviously it doesn't know anything about your runtime. It declares variables in the scope they're first assigned in; so 'global.x = y' works, while 'global = {}' compiles to 'var global; global = {};'.
CoffeeScript doesn't allow you to shadow variables in enclosing scopes. The compiler will declare the variable in the "outer-most" scope in which is is used and then any uses in enclosed scopes refer to that one.

In this case as long as OP was not using the variable in an enclosing scope, it would have been made local to the function.

And CS also wraps modules in closures (by default), so an outermost-scoped variable is still local to the script, whereas it would be global to all scripts in vanilla JS.
This is true for the most part, but there's a subtle edge case. Within a nested function, variables used in loops will redeclare within the inner function:

  ->
    outerVar = "outer"
    ->
      # this will output "undefined" 
      # as CoffeeScript has redeclared it for the loop:
      console.log outerVar 
      0 for outerVar in [0]
Huh -- that looks like a (new) bug to me. We should reuse the external declaration.
It always creates a local var. You can only assign to globals indirectly, through properties of the global object e.g. window.foo = 123.
(you either use var or val, making it easy to change from re-assignable variables to final ones)

I imagine that must be a headache for non-native speakers for whom "l" and "r" are indistinguishable. Imagine coding in a language developed in China with keywords "mā" and "mǎ"… most English-speaking developers would never remember which one to use because they can't distinguish between the two verbally.

Am I right in thinking that the javascript

  /* "use strict" */
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 ..
Yes:

  $ node
  > "use strict"; initial = "foo";
  ReferenceError: initial is not defined
      at repl:1:17
      at Interface.<anonymous> (repl.js:168:22)
      ...
Yeah, that's all he needed. Making it a comment isn't necessary either.

Maybe Node should be strict by default? I can understand it not being the case for browsers that need to support legacy code, but for Node it doesn't really need to care about that.

Huh. On the scale from pragmatic idiosyncrasy to simply bizarre, making a string literal expression change the runtime behaviour in that manner is fairly wide towards the bizarre end of the scale.

I think, if anything, requiring it to be in a comment would have been less odd; at least comments imply a sense of "meta"-ness.

Yeah, I always thought the string idea was wacko too, but introducing a new keyword in a language that has to be backwards compatible or suffer doom isn't a good idea either.
Comments are commonly stripped by various source code processors out there, including minifiers and some browser JS engine front-ends.

The string literal doesn't make anyone _happy_, but it was most compatible with the existing widely deployed toolchains, as I understand.

Not to mention that it reeks of VB6's "Option Explicit" and "Option Strict" statements. I hated those.
> On the scale from pragmatic idiosyncrasy to simply bizarre, making a string literal expression change the runtime behaviour in that manner is fairly wide towards the bizarre end of the scale.

Yes, on the other hand it's quite necessary to not have non-compliant browsers blow up. `"use strict";` is just a noop in e.g. IE6. `use strict;` would be an error in it.

> I think, if anything, requiring it to be in a comment would have been less odd; at least comments imply a sense of "meta"-ness.

Javascript compressors definitely strip comments. They probably don't strip strings.

Quite possibly a good idea.

  use v5.12;
in perl code enables strict, at least.
Node itself (the core js library) isn't strict mode compliant: it uses octal literals for filesystem interactions.
Running some variation of JSLint in his editor/IDE would likely have caught this, too.
Or using js2-mode in Emacs. That's just one small reason why Emacs in my JavaScript workhorse. I use TextMate for most things but if I'm writing JS all day nothing beats Emacs.
Yes, though never enable strict mode (in JS) unless you know exactly what it does. `use strict` causes some subtle semantic shifts that can be easily overlooked. For example, there's no dynamic binding between the `arguments` object and a function's arguments:

    ((x)-> x = 2; arguments[0] is 1)(1) # true

    ((x)-> arguments[0] = 2; x is 1)(1) # true

Any developer enabling strict mode should be testing heavily in browsers that actually support the feature. There have been a few high-profile meltdowns already (on BoA, Intel, eBay, etc.) due to devs not understanding the feature, or concatenating a `"use strict"` script (not wrapped in an IIFE) with non-strict code.

JSLint doesn't help matters, as it errors unless `"use strict"` is declared (unless run with the "sloppy" option -- something I'm sure devs love). The absolute worst scenario is for a browser to implement some kind of heuristics-based approach to respect the `"use strict"` prologue; what Brendan Eich calls a "strict quirks mode".

All that said, I'm a big fan of `strict`. It might be a good idea to use it in production, and strip the directives out when deploying.

Since the code in question was run server side using nodejs you can just enable it. Using it for client side javascript is another case indeed.
(comment deleted)
No.

    > var initial = 'string';
    >  "use strict"; (function(){ initial = 'anotherstring' })();
    > initial
    'anotherstring'
    >
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.
Indeed, JSLint or JSHint would have cought this. The claim that "nothing I could do in best practice ... would have caught the offending line" is false.
JSLint was my first response, but I have to agree with someone higher up -- use strict is probably a better solution, since it'll happen whether you like it (or forget it) or not.
using strict sounds like a great idea - I wonder what other implications that has, need to look into it.
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:

    for (x=0;x<variable.length;x++) {
    // blah
    }
I wasn't declaring the x variable, so it was using the x part of the x/y positioning of the UI container. Woops.

edit: initialise/declare brain freeze

Ha, that reminds me of a program I was writing in MATLAB. I used `i` for a loop index in one part of the script and then later when doing some complex number calculations (`2 + 3i`)...it was one of those so-stupid-you-have-to-laugh moments when I finally figured it out.
It's for this precise reason that I use 1i rather than i when doing complex arithmetic in matlab.
Wait... matlab allows "AB" to be inferred as "A*B"? How is that not a disaster? Parsing a token that doesn't even exist in the lexer stream? Even C++ wouldn't try that.
1i is special, sort of like 1d or 1f in C. The i refers to the imaginary number datatype. i also can be reassigned, which is a horrible design bug in Matlab.
Classic! I did that with rotation in my js gaming engine once, fun times.
I wasn't initialising the x variable

Do you mean s/initialising/declaring/, or am I just really confused?

He means declaring.
Yes, sorry, I meant declaring. Wrote that post in a hurry as I headed out the door, I wasn't paying too much attention.
Hmm, so you wrote a careless typo while describing your "careless" coding? Not to be harsh, but maybe it's a hint?
Well if you really want to go into painful detail about it, I had originally phrased the sentence differently, something along the lines of "leaving out the var in the for loop initialisation". It wasn't very readable so I changed it, but the word 'initialise' was on my mind.

In any case, I'm not sure that I need a hint to tell me that doing things quickly and carelessly is a bad idea. But we all do it sometimes, in both our programming and our writing.

Well sure. I actually wasn't trying to be harsh OR sarcastic. It's not pop psychology (I think it goes back to Freud) that in some sense there are no mistakes. We all make errors and I was just saying that two errors about the same issue might be a hint that one has "issues" about the, er, issue or that some self-sabotage in important matters might surface under stress. You're the only one can answer that. I admire your courage in sharing your experience with all these crack programmers.
Not to sound harsh, followed by something quite sarcastic and judgemental isn't a particularly nice way to address someone you don't know.

I for one would speculate, not to sound harsh, that if you haven't made silly errors like this then you haven't been around the block enough times.

> I for one would speculate, not to sound harsh, that if you haven't made silly errors like this then you haven't been around the block enough times.

on the other hand, those of us who have been around the block enough times have almost certainly encountered co-workers who were bright, talented, cute, whatever - but at the end of the day were just ... sloppy ... lazy ... careless ... in their approach. It's a personality thing - they had nice childhoods, they never learned to by hyper-vigilant and paranoid, and as a consequence... they write buggy code that can't be trusted.

"...never learned to by hyper-vigilant..."

My eyebrow just raised so far my forehead cramped.

Muphry's law.
I'm contemptuous of his coding practice, not his writing or grammar. His writing is pleasant to read... but so what? where are the multiple layers of defense standing between a simple syntactical or semantic error and a full scale business-impacting technical fuck up. that is what I meant by being vigilant and paranoid.
I'd be offended if it wasn't for the fact that your original accusation appears to be that I had had a happy childhood. I'll take that one on the chin.

As for the rest... well, I don't understand where you're going with it. Pop psychology aside, are you really suggesting that you write utterly seamless code every single time? You've never done a build and then realised that you made an error somewhere along the way, gone back and fixed it? You act as if my mistake had the potential to ruin a business. Of course it didn't- I picked up on it before the code had even been pushed to the remote repository.

You can live in a world where everyone does everything perfectly, every time (and pay the price when you inevitably don't) or you can set up systems with unit testing, user testing, and- yes- developer testing that results in bugs being dealt with in a timely manner before a single end-user sees anything.

But hey, each to their own. Whatever works for you. If you get it right first time, every time, then you are a better programmer than I, and I congratulate you on it.

>>You act as if my mistake had the potential to ruin a business. Of course it didn't- I picked up on it before the code had even been pushed to the remote repository.

Not to beat on you, but didn't you discover the error after users started using it in a big news day?

I think you misinterpreted my remark. This actually wasn't a "silly" error, it was, as many have pointed out, an avoidable error. I think many don't take javascript seriously and it bites them.
Now there's an argument for syntax highlighting.

It also doesn't happen in languages where you must say self.x to access a property/field called x.

Exactly my first thought: "This wouldn't happen with Python".
Only for case of instance variable. Python still has nested function, non-instance variable, and implicit declaration.

Only it is worse in Python because, lacking `var` keyword, you will shadow the variable in Python

Now that I understand what you're saying: This is one of the reasons I hate the "feature" in C++ and C99 that you can declare variables anywhere. If you stick to declaring all your variables in one place, it's much easier to notice when you've already used a variable name.
And much easier to wind up with lots of bad dead code or worse, reusing variables over and over leading to tightly coupled code. IMHO, variables should be declared as closely as possible to where they're used in the smallest scope possible. C languages do it right.
Have you tried the GCC -Wshadow warning flag? It will generate a warning whenever you define a variable with the same name as one declared in an outer scope.

Between that, and using -Werror to ensure that warnings never get ignored, I find C99-style declare-anywhere quite useful. It helps me keep the scope of variables limited to the places that need them.

That said, I rarely use C99's ability to declare a variable in the middle of a block. I primarily use the C99 feature of declaring variables as part of a loop.

(I'm no C expert, I'm posting my take on this mainly because I'm interested in learning from counterarguments):

Greatgrandparent's error was lack of such inner declaration. -Wshadow wouldn't help with that.

Also, I imagine enforcing old-style declaration restrictions helps you to avoid introducing those errors in first place, but it's just as hard to debug them once they're in.

My choice would be to get in the habit of declaring a variable in the narrowest scope that makes sense. I can't see much of a problem with this (esp. in the context of C!) barring old habit. I don't think Scheme programmers, for example, have this stuff any easier and I don't think scoping is perceived as a problem there.

I agree with you entirely that -Wshadow (or an equivalent for JavaScript) would not solve the problem reported in the original article. However, it does solve the problem cperciva mentioned in the comment I replied to: "If you stick to declaring all your variables in one place, it's much easier to notice when you've already used a variable name.". -Wshadow detects when you've already used a variable name and reports that as a warning (or an error if you use -Werror, which you should).

I also agree about declaring a variable in the narrowest scope that makes sense. I often see code that attempts to reuse the same variable for several different purposes, rather than declaring separate variables in narrower scopes.

Better to use this feature, but turn warnings on and treat all warnings as errors.
Umm, I find the opposite to be true. By always declaring variables where I use them (notably in for loops), I avoid accidentally reusing a variable I used elsewhere.
That is a special case. Loop headers should be considered the top of scope blocks in C. I don't know why the language designers didn't do that.
That makes variables scope over far more than they need to, opening up a pretty frequent bug avenue of using the wrong variable that you did not intend to use.
exactly what happened to me. I had two for loops defined in my JS code which used the variable "i" without declaring. Hence, when the code ran, the second for loop behaving weirdly. It took sometime before catching the bug.
PHP experience can poison development in other languages.
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
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.

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.

Been using JSHint ever since.

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.

[1]: https://github.com/mooz/js2-mode

I was going to say the same thing, it also makes me avoid missing semicolons by making those errors feel like sores in my code. I can't let them mess things up they need to be gone before I can write another line.
Have you had any luck getting it to handle modern JavaScript style? Whenever I've tried js2-mode it really hasn't liked jQuery style nested anonymous functions.
Do you have a specific example in mind? It works pretty well for me.
Try using the version I linked rather than the original. It has a bunch of improvements including how it handles indenting functions.

I've done some moderately complicated jQuery development with it, as well as some node.js stuff for fun, and have had no issues in either case.

Ouch.

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.

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.

As a rule, in js I declare every var at the top of the function (or globe) in one giant var statement just because of this. Still, it's easy to get lost in the moment and mess up just once. I used to think this was just the way things were but pair-programming/code-review and/or a language with better scoping rules would've prevented this from happening.
> Still, it's easy to get lost in the moment and mess up just once.

Get a better editor, and JSHint, and "use strict";

Implicitly declared globals can be statically checked, there is no reason not to.

No such thing as "global-variables-by-default".

    > (function () { internalvariablewithnovardelcaration = "string"; })();
    > internalvariablewithnovardeclaration
    ReferenceError: internalvariablewithnovardeclaration is not defined
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 yourself a favor and use CoffeeScript! You will never have to worry about this or any number of other trivial JS mistakes, plus your code will be more readable and more fun to write. Win/win.
I just hit your page and it gave me an error that said the server was over capacity. Not the only reason?
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).
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.
Screw rounding up >1 users for testing (though it does give needed human insight), if you aren't spooling up a few multicore VMs with JMeter or better, you aren't really performance testing.
Common problem. Live and learn.

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.

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.

(comment deleted)
> 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.

jshint would have caught it. You need to run jshint on your code or you will get silly errors like this. Simple.

Even better is setting your editor to run JSHint when you save a .js file, and let you know if there are problems. Not only does it avoid stupid bugs, it saves time round-tripping to the browser for trivial issues like syntax errors.
Sounds like a good idea. I know emacs flymake mode can be set up to underline problems detected by jshint. Personally I like to have the tests and jshint run by a hotkey so I can happily move the code through invalid states (towards a valid goal) without being constantly complained at :)
Never heard of JSHint. Thanks for the tip!
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?
Well whole rocket launches failed for silly reasons like temperature conversions, so you shouldn't feel too bad about it.
This site a scam? Create a fake account using a completely random name, it seems to have found a lot about "shpadoinkle Hassenpheffer" on the internet, I however can't find a single reference to the name...
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.
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).
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.

This technique totally went to shit when I came across one of our scripts that gratuitously used `apply()` all over the place.

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.

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. :)

if you want to filter out prototypes you can use object.hasOwnProperty( property )

    > also goes through the prototype methods after the
    > elements.
That shouldn't be happening. If you see it happening, your JS implementation is just buggy.

In fact, this testcase:

    <script>
      window.onload = function() {
        var arr = window[0].arr;
        for (var i in arr) { alert(i); }
      }
    </script>
    <iframe src="data:text/html,<script>arr=[1];</script>">
    </iframe>
alerts only "0" in WebKit+JSC, Gecko, Presto. Chrome has some sort of bizarre security policy here that keeps the script from working, so no idea what WebKit+V8 does.
I'm not an expert in JavaScript...

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 Javascript: The Good Parts. 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.

Honestly though, I've tried jslint and jshint and a bunch of other lint-like tools on the Haraka source code (google it), and it barfs all over the place for no particular reason.

"use strict" is catching a lot more errors, though frustratingly enough, not until runtime.

Excellent advice. You simply have to understand JS scope rules, and the Crockford post lays it out compactly. It's an idiosyncratic language.
I think the author of the blog post understands JS scope rules. His point isn't that you shouldn't need to have a "var" in front of your declarations or that javascript is stupid and he doesn't understand it, it's that there should be a better way to find bugs like the one that bit him.

Of course, there is a way to find his particular bug (JSLint, "use strict") that he didn't know about. Still, the point about debugging being a nightmare is absolutely true.

After I posted, I realized this. You're correct.
Javascript Patterns by Stoyan Stefanov is another great primer on JS best practices. Above all, if you're not delinting and using strict, you're dead in the water.
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.

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.

this question can be found in the first chapter of every good JavaScript Book: What is the difference between a and b in the following statement?

a = b = 0;

In JavaScript you CAN define a variable without var, but then it becomes global. As code is read from the right side, b is assigned 0, but b is a variable now, so a is assigned to a variable and is not global then. The difference is: b is global and a is local. What do we learn from this? NEVER forget var unless there is a reason ..