13 ms·

How One Missing `var` Ruined our Launch

317 points by Trindaz 15y ago

mnutt 15y ago
This is why I really dislike it when I see var a = "foo", b = "bar", c = "baz"; All it takes is one missed comma and all of a sudden you've turned a lot of your variables global.
- fuzzythinker 15y ago
  that's why many people favor this variant: var a = "foo" , b = "bar" , c = "baz" ;
  - webXL 15y ago
    I see this hideous convention used a lot when assigning nodejs modules, so you can easily comment out modules for debugging purposes (I think). Other than that, why is "[NL], b = 1" safer or better than "b = 1,[NL]"?
  - mnutt 15y ago
    That makes the commas easier to see (and is ugly IMO) but it's still one comma away from making the rest of your variables global. What's so bad about just var a = "foo"; var b = "bar"; var c = "baz";
DanielRibeiro 15y ago
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/ http://js2coffee.org/
- finnw 15y ago
  I don't know Coffeescript - does it implicitly create a local var when there is already a global with the same name?
  - Cushman 15y ago
    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 = {};'.
  - YmMot 15y ago
    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.
    - mmahemoff 15y ago
      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.
    - geraldalewis 15y ago
      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]
      - jashkenas 15y ago
        Huh -- that looks like a (new) bug to me. We should reuse the external declaration.
mst 15y ago
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 ..
- apaprocki 15y ago
  Yes: $ node > "use strict"; initial = "foo"; ReferenceError: initial is not defined at repl:1:17 at Interface.<anonymous> (repl.js:168:22) ...
- Raticide 15y ago
  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.
  - morsch 15y ago
    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.
    - apgwoz 15y ago
      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.
    - bzbarsky 15y ago
      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.
dikbrouwer 15y ago
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.
- Waquo 15y ago
  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.
- NegativeK 15y ago
  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.
  - dikbrouwer 15y ago
    using strict sounds like a great idea - I wonder what other implications that has, need to look into it.
untog 15y ago
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
- swanson 15y ago
  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.
  - joeyo 15y ago
    It's for this precise reason that I use 1i rather than i when doing complex arithmetic in matlab.
    - ajross 15y ago
      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.
      - gujk 15y ago
        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.
- vonkow 15y ago
  Classic! I did that with rotation in my js gaming engine once, fun times.
howardr 15y ago
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
mstepniowski 15y ago
Use jslint, Luke.
itay 15y ago
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.
tikhonj 15y ago
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 https://github.com/mooz/js2-mode
- nkassis 15y ago
  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.
- almost 15y ago
  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.
  - gcv 15y ago
    Do you have a specific example in mind? It works pretty well for me.
  - tikhonj 15y ago
    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.
JonnieCache 15y ago
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.
- vonkow 15y ago
  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.
  - masklinn 15y ago
    > 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.
- chrisrhoden 15y ago
  No such thing as "global-variables-by-default". > (function () { internalvariablewithnovardelcaration = "string"; })(); > internalvariablewithnovardeclaration ReferenceError: internalvariablewithnovardeclaration is not defined
crichardson917 15y ago
Very interesting. I've been thinking about delving into Node.js for a while, and I will definitely keep things like this in mind.
- samgro 15y ago
  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.
atomical 15y ago
I just hit your page and it gave me an error that said the server was over capacity. Not the only reason?
- ricardobeat 15y ago
  That's Tumblr apparently.
vonkow 15y ago
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).
Hominem 15y ago
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.
- vonkow 15y ago
  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.
jconley 15y ago
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.
deleted 15y ago
[deleted]
almost 15y ago
> 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.
- apike 15y ago
  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.
  - almost 15y ago
    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 :)
    - daleharvey 15y ago
      I wrote a jshint mode for emacs https://github.com/daleharvey/jshint-mode https://github.com/daleharvey/jshint-mode
      - wahnfrieden 15y ago
        I use jshint in vim as well: https://github.com/wookiehangover/jshint.vim https://github.com/wookiehangover/jshint.vim
        eric-hu 15y ago
        does this work for coffeescript as well?
        15y ago
peregrine 15y ago
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?
Tichy 15y ago
Well whole rocket launches failed for silly reasons like temperature conversions, so you shouldn't feel too bad about it.
tastynachos 15y ago
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...
codejoust 15y ago
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.
dap 15y ago
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).
FuzzyDunlop 15y ago
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. :)
- ruethewhirled 15y ago
  if you want to filter out prototypes you can use object.hasOwnProperty( property )
- bzbarsky 15y ago
  > 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.
mzarate06 15y ago
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.
- baudehlo 15y ago
  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.
- mturmon 15y ago
  Excellent advice. You simply have to understand JS scope rules, and the Crockford post lays it out compactly. It's an idiosyncratic language.
  - sirclueless 15y ago
    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.
    - mturmon 15y ago
      After I posted, I realized this. You're correct.
- smokestack 15y ago
  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.
ConstantineXVI 15y ago
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.
coldarchon 15y ago
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 ..
mansoor-s 15y ago
"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." Perfect example of a developer hating JS just because he/she didn't bother to learn it first and got burned.
- mhansen 15y ago
  It's still very easy to miss a 'var', even if you know the language. Especially if you've just context-switched from ruby or python.
jessedhillon 15y ago
Do I understand this right -- global variables are shared across all requests in node.js? If so, that is an insane design.
- tikhonj 15y ago
  It's not really a fault of node so much as a fault of JavaScript. While JavaScript is a beautiful and expressive language, it does have some annoying pitfalls; this is just one of them. Happily, this is all changing--in the long run, future versions of JavaScript should polish away these issues while maintaining the fundamentally sound core of the language. In the short run "use strict" lets you realize some of the upcoming improvements in otherwise legacy code.
- dbattaglia 15y ago
  Node is single-threaded, so yes they are shared. Requests are basically just multiplexed inside the same message loop. In my opinion it wouldn't be quite as scary if js didn't make it so damn simple to accidentally define a global variable (like in this instance, where it was never actually declared in globally-scoped code).
  - bzbarsky 15y ago
    Why does threading matter? You can have multiple JS global objects all running on the same thread. See any web browser.
    - pbz 15y ago
      JS in browsers is not multi-threaded. There's only one JS thread where requests from async calls are queued: http://ejohn.org/blog/how-javascript-timers-work/ http://ejohn.org/blog/how-javascript-timers-work/ When dealing with true multi-threading you have all sorts of issues you need to worry about, like locking for writing, worrying about deadlocks, etc. that make coding much more difficult.
      - bzbarsky 15y ago
rayiner 15y ago
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. Scheme got it right sometime in the 1970's. (let ((x initial-value)) ; binding (set! x new-value)) ; assignment Or in infix syntax (Dylan): let x = initial-value ; x := new-value ;
- cpeterso 15y ago
  ALGOL '58 used := and = as distinct operators for assignment and equality in 1958. https://en.wikipedia.org/wiki/ALGOL_58#ALGOL_58.27s_influence_on_ALGOL_60 https://en.wikipedia.org/wiki/ALGOL_58#ALGOL_58.27s_influenc...
- chromatic 15y ago
  My theory is that most of the offending language designers looked at variable declaration in languages with static typing and thought "Wait, we don't have static types, so there's no reason to declare variables!"
  - JoshTriplett 15y ago
    Personally, I like the opposite solution, used in functional languages: "Wait, we don't allow changing variables once created, so we only need to support variable initialization with a value, not reassignment of a variable or declaration without initialization!"
    - masklinn 15y ago
      > used in functional languages Used in some. Haskell and OCaml mandate scope binding (unless you're at the top level of a module). Erlang does single-assignment (really pattern matches aliasing, an extremely limited version of Prolog's unification) without scope declaration, but what other FP languages do that?
- MostAwesomeDude 15y ago
  Python doesn't let you write outside of local scope without a special keyword. Exactly the inverse of JS. If you want to write to global state, and you're aware that you're writing to global state, then you use the "global" keyword. That's all.
kaffeinecoma 15y ago
You can do some poor man's load testing very simply with siege (http://www.joedog.org/index/siege-home http://www.joedog.org/index/siege-home). Put a bunch of URLs in a text file, and run it with "siege -f urls.txt". Of course it doesn't prove 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.
jroseattle 15y ago
> 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. Sorry, that's incorrect. If you cannot fully simulate your environment for purposes of validation, you're not covering your bases.
- wanorris 15y ago
  Are you saying that the definition of an adequate test setup is one that will necessarily catch any possible race conditions? That sounds like a necessary idea if you're building real-time embedded systems for aircraft operation or something, but for a lot of applications, developing a test harness to that standard would take considerably more time and effort than building out the actual product.
  - jroseattle 15y ago
    The problem wasn't a race condition, it was an improperly scoped variable. If a different variable name had been used, the failure would have been easy to identify. Testing for the result of something improper wouldn't be appropriate here, either. But, if the environment is such that it is easy to manifest this condition, I would argue that such a test setup should definitely be considered, given the potential for failure.
fashionfshi 15y ago
Welcome to ==== http://www.fashion-long-4biz.com http://www.fashion-long-4biz.com == Air Jordan (1-24) shoes $35 UGG BOOT $50 Nike shox (R4, NZ, OZ, TL1, TL2, TL3) $35 Handbags ( Coach Lv fendi D&G) $35 T-shirts (polo, ed hardy, lacoste) $16 Jean (True Religion, ed hardy, coogi)$34 Sunglasses ( Oakey, coach, Gucci, Armaini)$15 New era cap $16 Bikini (Ed hardy, polo) $18 FREE SHIPPING http://www.fashion-long-4biz.com http://www.fashion-long-4biz.com
jhuni 15y ago
Always use JSLINT.
SimHacker 15y ago
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? 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?
- mark_story 15y ago
  No, because this is still valid inside the anonymous function. It can't tell which this you intended to get. It would be able to find out if you used an uninitialized me, _this, that though and complain. Using self as the this alias is really dangerous as it leaks to window.self so avoid it like the plague.
firefoxman1 15y ago
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.
jufo 15y ago
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.
bryantchou 15y ago
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.
igorgue 15y ago
I don't always write JavaScript (CoffeeScript FTW) but when I do I use JSLint (vim-jslint)...
- baudehlo 15y ago
  It's not very good on node.js code. Hell, it's not very good, period.
  - igorgue 15y ago
    The plugin uses node as a linter... let $JS_CMD='node'
kwamenum86 15y ago
"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." 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". http://www.jslint.com/ http://www.jslint.com/
rajeshamara 15y ago
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
- bobralian 15y ago
  There's a lot to say here, but I'll limit my comment to LOL at the idea that IE reports the exact line number of a javascript error.
- expertmind 15y ago
  Yes, These features are there in Firefox can Chrome. You clearly haven't used these browsers. I will suggest you to google for firebug, and developer tools in Chrome.
- thomblake 15y ago
  The funny thing about this comment is that you think he can use Internet Explorer to check his server-side code.
gambler 15y ago
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. 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.
startupcto 15y ago
"It’s a damn tragedy. I’m not an expert in JavaScript" 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. 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. I'm just saying, no hard feelings. :)
- sirclueless 15y ago
  I think "a damn tragedy" is a pretty apt description. Well-designed languages shouldn't require reams of experience. Just a little intelligence and a reference for any questions that arise should get you pretty far. I'm sure this guy "really understand[s] scoping in JS", but he still got bitten. The blog post isn't frivolous, its point is that javascript is a minefield. Which is an important lesson. Use JSLint or "use strict" or pay the price.
scotty79 15y ago
Php shares nothing except session between requests and local scope is default.
speleding 15y ago
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).
- thomblake 15y ago
  No IDE needed - any decent editor will do syntax highlighting.
  - speleding 15y ago
    Global versus local variables is not determined by syntax in javascript, it's determined by semantics (you need to parse javascript to find out). A few editors can do that, but you could really call those an IDE already.
    - thomblake 15y ago
      Parsing is syntax. Interpretation is semantics. Just parsing JS just gives you information about its syntax, and any decent editor (like vim or emacs) has a programmable interface such that you can write a JS parser.
ozataman 15y ago
Thanks for reminding me why I like Haskell so much.
brendoncrawford 15y ago
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... 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
- xentronium 15y ago
  Sure, blame the victim, not the idea that default variable scope is global.
  - viraptor 15y ago
    There are libraries in other languages providing node-like capabilities. He could use those and not have the global scope problem. Not saying that the idea makes any sense either...
  - ams6110 15y ago
    Every language and platform has idiosyncrasies that you ignore at your peril. You choose to use JavaScript, you live with its warts.
- dgabriel 15y ago
  Really smart, careful people also make mistakes. This is one of the things that can easily happen to a bright but inexperienced developer. While the numbered list is helpful, I don't think calling the programmer in question lazy, careless, or "just out to make a buck," is ok or fair.
ruidlopes 15y ago
(Shameless plug, but interesting enough, imho). This is why I created scopeleaks: https://github.com/ruidlopes/scopeleaks https://github.com/ruidlopes/scopeleaks
dendory 15y ago
Screams lack of testing. Speaking of which, did you know your site design breaks when viewed on an iPad?
yaix 15y ago
Wow, thanks for sharing! 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.
johnbender 15y ago
Steve Yeggae's js2-mode highlights globals during editing, and it's saved me a good many times. http://code.google.com/p/js2-mode/ http://code.google.com/p/js2-mode/ Mind you it won't catch globals declared in chained assignments eg var x = y = 0; // y is defined globally
- webXL 15y ago
  If you're looking for a new IDE, JetBrains WebStorm does this. In the chained assignment, as well.
- thomblake 15y ago
  The fork of js2-mode at https://github.com/mooz/js2-mode https://github.com/mooz/js2-mode will catch that, as will js3-mode.
jwatte 15y ago
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! 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)
ihatelowercase 15y ago
one word: jslint
chetan51 15y ago
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. Sorry to hear about your tragedy, hope it doesn't cause too many issues in the long run.
jwallaceparker 15y ago
Crazy. Same thing happened to me. Good luck with the next project.
jwallaceparker 15y ago
Crazy. Same thing happened to me. Good luck with the next project.
phektus 15y ago
"It’s beautiful simple..." 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).
sampsonjs 15y ago
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.
alexchamberlain 15y ago
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.
hunterp 15y ago
Bad Title. There is no such thing as a ruined launch. Launches don't matter. Adapt from failures constantly, and you will be just fine.
nikcub 15y ago
You will probably get hundreds of comments with unsolocited advice, but let me just say to checkout and setup JSLint: https://github.com/douglascrockford/JSLint https://github.com/douglascrockford/JSLint Setup and how you use it is more important than just using it. I run it in three places: 1) In my IDE (bound in vim on save, same in TextMate) 2) As a Git checking hook 3) In deployment / build scripts Turn all the warnings way up and it always catches redeclaration bugs. Set it up once and don't allow any code to get checked in or deployed with even a single warning present. 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. (if anybody is interested in how to set this all up I can publish my configs and scripts)
- philbo 15y ago
  I'd love to see them. I'm starting a new job soon and it is going to be pretty much greenfield javascript development all the way. I'm really keen to make sure that I'm ticking every box on the best practices front, right from day one.
- siculars 15y ago
  Please do share.
boscomutunga 15y ago
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.
nt_mark 15y ago
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?
poundy 15y ago
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?
illumen 15y ago
jslint
markokocic 15y ago
jslint to the rescue. If all your source is JS, there's no excuse to at least use lint.
JayInt 15y ago
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!!
deleted 15y ago
[deleted]
jcromartie 15y ago
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?
jorangreef 15y ago
for (var key in this) console.log(key);
kevinkemp 15y ago
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.
CrabDude 15y ago
For handling asynchronous exceptions and bad stack traces, checkout the "trycatch" module on npm, http://github.com/crabdude/trycatch http://github.com/crabdude/trycatch . It's an asynchronous try catch. trycatch(function(){ process.nextTick(function() { throw new Error("This will be caught."); }); }, function(e) { console.log(e.stack); }); I also integrated it into the step module to create "stepup" which adds error coalescing, http://github.com/crabdude/stepup http://github.com/crabdude/stepup .
gopi 15y ago
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). 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 :)
ighost 15y ago
> I would posit here that nothing I could do in best practice would have caught the offending line. bullshit. jslint or clojure compiler would have found this error at "compile time." coffeescript would have made it a non-issue.
chrisrhoden 15y ago
> (function () { internalvariablewithnovardelcaration = "string"; })(); > internalvariablewithnovardeclaration ReferenceError: internalvariablewithnovardeclaration is not defined at [object Context]:1:1 at Interface.<anonymous> (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.<anonymous> (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) > externalwithnovar = 1; 1 > (function () { externalwithnovar = "string"; })(); > externalwithnovar 'string' 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.

317 points by Trindaz 15y ago

mnutt 15y ago

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

fuzzythinker 15y ago
that's why many people favor this variant: var a = "foo" , b = "bar" , c = "baz" ;
- webXL 15y ago
  I see this hideous convention used a lot when assigning nodejs modules, so you can easily comment out modules for debugging purposes (I think). Other than that, why is "[NL], b = 1" safer or better than "b = 1,[NL]"?
- mnutt 15y ago
  That makes the commas easier to see (and is ugly IMO) but it's still one comma away from making the rest of your variables global. What's so bad about just var a = "foo"; var b = "bar"; var c = "baz";

DanielRibeiro 15y ago

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/ http://js2coffee.org/

finnw 15y ago
I don't know Coffeescript - does it implicitly create a local var when there is already a global with the same name?
- Cushman 15y ago
  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 = {};'.
- YmMot 15y ago
  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.
  - mmahemoff 15y ago
    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.
  - geraldalewis 15y ago
    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]
    - jashkenas 15y ago
      Huh -- that looks like a (new) bug to me. We should reuse the external declaration.

mst 15y ago

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

apaprocki 15y ago
Yes: $ node > "use strict"; initial = "foo"; ReferenceError: initial is not defined at repl:1:17 at Interface.<anonymous> (repl.js:168:22) ...
Raticide 15y ago
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.
- morsch 15y ago
  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.
  - apgwoz 15y ago
    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.
  - bzbarsky 15y ago
    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.

dikbrouwer 15y ago

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.

Waquo 15y ago
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.
NegativeK 15y ago
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.
- dikbrouwer 15y ago
  using strict sounds like a great idea - I wonder what other implications that has, need to look into it.

untog 15y ago

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

swanson 15y ago
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.
- joeyo 15y ago
  It's for this precise reason that I use 1i rather than i when doing complex arithmetic in matlab.
  - ajross 15y ago
    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.
    - gujk 15y ago
      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.
vonkow 15y ago
Classic! I did that with rotation in my js gaming engine once, fun times.

howardr 15y ago

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

mstepniowski 15y ago

Use jslint, Luke.

itay 15y ago

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.

tikhonj 15y ago

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 https://github.com/mooz/js2-mode

nkassis 15y ago
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.
almost 15y ago
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.
- gcv 15y ago
  Do you have a specific example in mind? It works pretty well for me.
- tikhonj 15y ago
  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.

JonnieCache 15y ago

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.

vonkow 15y ago
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.
- masklinn 15y ago
  > 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.
chrisrhoden 15y ago
No such thing as "global-variables-by-default". > (function () { internalvariablewithnovardelcaration = "string"; })(); > internalvariablewithnovardeclaration ReferenceError: internalvariablewithnovardeclaration is not defined

crichardson917 15y ago

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

samgro 15y ago
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.

atomical 15y ago

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

ricardobeat 15y ago
That's Tumblr apparently.

vonkow 15y ago

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

Hominem 15y ago

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.

vonkow 15y ago
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.

jconley 15y ago

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.

deleted 15y ago

[deleted]

almost 15y ago

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

apike 15y ago
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.
- almost 15y ago
  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 :)
  - daleharvey 15y ago
    I wrote a jshint mode for emacs https://github.com/daleharvey/jshint-mode https://github.com/daleharvey/jshint-mode
    - wahnfrieden 15y ago
      I use jshint in vim as well: https://github.com/wookiehangover/jshint.vim https://github.com/wookiehangover/jshint.vim
      - eric-hu 15y ago
        does this work for coffeescript as well?
        15y ago

peregrine 15y ago

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?

Tichy 15y ago

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

tastynachos 15y ago

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

codejoust 15y ago

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.

dap 15y ago

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

FuzzyDunlop 15y ago

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

ruethewhirled 15y ago
if you want to filter out prototypes you can use object.hasOwnProperty( property )
bzbarsky 15y ago
> 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.

mzarate06 15y ago

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.

baudehlo 15y ago
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.
mturmon 15y ago
Excellent advice. You simply have to understand JS scope rules, and the Crockford post lays it out compactly. It's an idiosyncratic language.
- sirclueless 15y ago
  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.
  - mturmon 15y ago
    After I posted, I realized this. You're correct.
smokestack 15y ago
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.

ConstantineXVI 15y ago

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.

coldarchon 15y ago

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

mansoor-s 15y ago

"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." Perfect example of a developer hating JS just because he/she didn't bother to learn it first and got burned.

mhansen 15y ago
It's still very easy to miss a 'var', even if you know the language. Especially if you've just context-switched from ruby or python.

jessedhillon 15y ago

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

tikhonj 15y ago
It's not really a fault of node so much as a fault of JavaScript. While JavaScript is a beautiful and expressive language, it does have some annoying pitfalls; this is just one of them. Happily, this is all changing--in the long run, future versions of JavaScript should polish away these issues while maintaining the fundamentally sound core of the language. In the short run "use strict" lets you realize some of the upcoming improvements in otherwise legacy code.
dbattaglia 15y ago
Node is single-threaded, so yes they are shared. Requests are basically just multiplexed inside the same message loop. In my opinion it wouldn't be quite as scary if js didn't make it so damn simple to accidentally define a global variable (like in this instance, where it was never actually declared in globally-scoped code).
- bzbarsky 15y ago
  Why does threading matter? You can have multiple JS global objects all running on the same thread. See any web browser.
  - pbz 15y ago
    JS in browsers is not multi-threaded. There's only one JS thread where requests from async calls are queued: http://ejohn.org/blog/how-javascript-timers-work/ http://ejohn.org/blog/how-javascript-timers-work/ When dealing with true multi-threading you have all sorts of issues you need to worry about, like locking for writing, worrying about deadlocks, etc. that make coding much more difficult.
    - bzbarsky 15y ago

rayiner 15y ago

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. Scheme got it right sometime in the 1970's. (let ((x initial-value)) ; binding (set! x new-value)) ; assignment Or in infix syntax (Dylan): let x = initial-value ; x := new-value ;

cpeterso 15y ago
ALGOL '58 used := and = as distinct operators for assignment and equality in 1958. https://en.wikipedia.org/wiki/ALGOL_58#ALGOL_58.27s_influence_on_ALGOL_60 https://en.wikipedia.org/wiki/ALGOL_58#ALGOL_58.27s_influenc...
chromatic 15y ago
My theory is that most of the offending language designers looked at variable declaration in languages with static typing and thought "Wait, we don't have static types, so there's no reason to declare variables!"
- JoshTriplett 15y ago
  Personally, I like the opposite solution, used in functional languages: "Wait, we don't allow changing variables once created, so we only need to support variable initialization with a value, not reassignment of a variable or declaration without initialization!"
  - masklinn 15y ago
    > used in functional languages Used in some. Haskell and OCaml mandate scope binding (unless you're at the top level of a module). Erlang does single-assignment (really pattern matches aliasing, an extremely limited version of Prolog's unification) without scope declaration, but what other FP languages do that?
MostAwesomeDude 15y ago
Python doesn't let you write outside of local scope without a special keyword. Exactly the inverse of JS. If you want to write to global state, and you're aware that you're writing to global state, then you use the "global" keyword. That's all.

kaffeinecoma 15y ago

You can do some poor man's load testing very simply with siege (http://www.joedog.org/index/siege-home http://www.joedog.org/index/siege-home). Put a bunch of URLs in a text file, and run it with "siege -f urls.txt". Of course it doesn't prove 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.

jroseattle 15y ago

> 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. Sorry, that's incorrect. If you cannot fully simulate your environment for purposes of validation, you're not covering your bases.

wanorris 15y ago
Are you saying that the definition of an adequate test setup is one that will necessarily catch any possible race conditions? That sounds like a necessary idea if you're building real-time embedded systems for aircraft operation or something, but for a lot of applications, developing a test harness to that standard would take considerably more time and effort than building out the actual product.
- jroseattle 15y ago
  The problem wasn't a race condition, it was an improperly scoped variable. If a different variable name had been used, the failure would have been easy to identify. Testing for the result of something improper wouldn't be appropriate here, either. But, if the environment is such that it is easy to manifest this condition, I would argue that such a test setup should definitely be considered, given the potential for failure.

fashionfshi 15y ago

Welcome to ==== http://www.fashion-long-4biz.com http://www.fashion-long-4biz.com == Air Jordan (1-24) shoes $35 UGG BOOT $50 Nike shox (R4, NZ, OZ, TL1, TL2, TL3) $35 Handbags ( Coach Lv fendi D&G) $35 T-shirts (polo, ed hardy, lacoste) $16 Jean (True Religion, ed hardy, coogi)$34 Sunglasses ( Oakey, coach, Gucci, Armaini)$15 New era cap $16 Bikini (Ed hardy, polo) $18 FREE SHIPPING http://www.fashion-long-4biz.com http://www.fashion-long-4biz.com

jhuni 15y ago

Always use JSLINT.

SimHacker 15y ago

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? 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?

mark_story 15y ago
No, because this is still valid inside the anonymous function. It can't tell which this you intended to get. It would be able to find out if you used an uninitialized me, _this, that though and complain. Using self as the this alias is really dangerous as it leaks to window.self so avoid it like the plague.

firefoxman1 15y ago

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.

jufo 15y ago

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.

bryantchou 15y ago

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.

igorgue 15y ago

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

baudehlo 15y ago
It's not very good on node.js code. Hell, it's not very good, period.
- igorgue 15y ago
  The plugin uses node as a linter... let $JS_CMD='node'

kwamenum86 15y ago

"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." 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". http://www.jslint.com/ http://www.jslint.com/

rajeshamara 15y ago

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

bobralian 15y ago
There's a lot to say here, but I'll limit my comment to LOL at the idea that IE reports the exact line number of a javascript error.
expertmind 15y ago
Yes, These features are there in Firefox can Chrome. You clearly haven't used these browsers. I will suggest you to google for firebug, and developer tools in Chrome.
thomblake 15y ago
The funny thing about this comment is that you think he can use Internet Explorer to check his server-side code.

gambler 15y ago

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

startupcto 15y ago

"It’s a damn tragedy. I’m not an expert in JavaScript" 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. 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. I'm just saying, no hard feelings. :)

sirclueless 15y ago
I think "a damn tragedy" is a pretty apt description. Well-designed languages shouldn't require reams of experience. Just a little intelligence and a reference for any questions that arise should get you pretty far. I'm sure this guy "really understand[s] scoping in JS", but he still got bitten. The blog post isn't frivolous, its point is that javascript is a minefield. Which is an important lesson. Use JSLint or "use strict" or pay the price.

scotty79 15y ago

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

speleding 15y ago

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

thomblake 15y ago
No IDE needed - any decent editor will do syntax highlighting.
- speleding 15y ago
  Global versus local variables is not determined by syntax in javascript, it's determined by semantics (you need to parse javascript to find out). A few editors can do that, but you could really call those an IDE already.
  - thomblake 15y ago
    Parsing is syntax. Interpretation is semantics. Just parsing JS just gives you information about its syntax, and any decent editor (like vim or emacs) has a programmable interface such that you can write a JS parser.

ozataman 15y ago

Thanks for reminding me why I like Haskell so much.

brendoncrawford 15y ago

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

xentronium 15y ago
Sure, blame the victim, not the idea that default variable scope is global.
- viraptor 15y ago
  There are libraries in other languages providing node-like capabilities. He could use those and not have the global scope problem. Not saying that the idea makes any sense either...
- ams6110 15y ago
  Every language and platform has idiosyncrasies that you ignore at your peril. You choose to use JavaScript, you live with its warts.
dgabriel 15y ago
Really smart, careful people also make mistakes. This is one of the things that can easily happen to a bright but inexperienced developer. While the numbered list is helpful, I don't think calling the programmer in question lazy, careless, or "just out to make a buck," is ok or fair.

ruidlopes 15y ago

(Shameless plug, but interesting enough, imho). This is why I created scopeleaks: https://github.com/ruidlopes/scopeleaks https://github.com/ruidlopes/scopeleaks

dendory 15y ago

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

yaix 15y ago

Wow, thanks for sharing! 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.

johnbender 15y ago

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

webXL 15y ago
If you're looking for a new IDE, JetBrains WebStorm does this. In the chained assignment, as well.
thomblake 15y ago
The fork of js2-mode at https://github.com/mooz/js2-mode https://github.com/mooz/js2-mode will catch that, as will js3-mode.

jwatte 15y ago

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! 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)

ihatelowercase 15y ago

one word: jslint

chetan51 15y ago

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. Sorry to hear about your tragedy, hope it doesn't cause too many issues in the long run.

jwallaceparker 15y ago

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

jwallaceparker 15y ago

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

phektus 15y ago

"It’s beautiful simple..." 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).

sampsonjs 15y ago

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.

alexchamberlain 15y ago

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.

hunterp 15y ago

Bad Title. There is no such thing as a ruined launch. Launches don't matter. Adapt from failures constantly, and you will be just fine.

nikcub 15y ago

You will probably get hundreds of comments with unsolocited advice, but let me just say to checkout and setup JSLint: https://github.com/douglascrockford/JSLint https://github.com/douglascrockford/JSLint Setup and how you use it is more important than just using it. I run it in three places: 1) In my IDE (bound in vim on save, same in TextMate) 2) As a Git checking hook 3) In deployment / build scripts Turn all the warnings way up and it always catches redeclaration bugs. Set it up once and don't allow any code to get checked in or deployed with even a single warning present. 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. (if anybody is interested in how to set this all up I can publish my configs and scripts)

philbo 15y ago
I'd love to see them. I'm starting a new job soon and it is going to be pretty much greenfield javascript development all the way. I'm really keen to make sure that I'm ticking every box on the best practices front, right from day one.
siculars 15y ago
Please do share.

boscomutunga 15y ago

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.

nt_mark 15y ago

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?

poundy 15y ago

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?

illumen 15y ago

jslint

markokocic 15y ago

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

JayInt 15y ago

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!!

deleted 15y ago

[deleted]

jcromartie 15y ago

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?

jorangreef 15y ago

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

kevinkemp 15y ago

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.

CrabDude 15y ago

For handling asynchronous exceptions and bad stack traces, checkout the "trycatch" module on npm, http://github.com/crabdude/trycatch http://github.com/crabdude/trycatch . It's an asynchronous try catch. trycatch(function(){ process.nextTick(function() { throw new Error("This will be caught."); }); }, function(e) { console.log(e.stack); }); I also integrated it into the step module to create "stepup" which adds error coalescing, http://github.com/crabdude/stepup http://github.com/crabdude/stepup .

gopi 15y ago

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

ighost 15y ago

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

chrisrhoden 15y ago

> (function () { internalvariablewithnovardelcaration = "string"; })(); > internalvariablewithnovardeclaration ReferenceError: internalvariablewithnovardeclaration is not defined at [object Context]:1:1 at Interface.<anonymous> (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.<anonymous> (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) > externalwithnovar = 1; 1 > (function () { externalwithnovar = "string"; })(); > externalwithnovar 'string' 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.