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, 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.
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.
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.
> 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.
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.
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.
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.
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.
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.
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.
> 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.
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?
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.
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 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.
> 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 :)
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.
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.
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/
(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.
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.
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]
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.
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 = {};'.
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.
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.
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.
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.
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
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!"
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!"
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?
Io has a similar distinction. In the Io language, := is syntax sugar for newSlot and = only means updateSlot. So if you accidently = a slot that doesn't exist, it doesn't new-up a magic global, it raises an error.
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.
Exactly. The claim is that implicit local variables are less dangerous than implicit global variables. I've been bitten badly by both. Perl was the worst, where implicit variables were scoped differently according to Larry's philosophy when he implemented it. All for the sake of brevity.
Can someone please implement strict mode for Node?
Python implicitly binds to the innermost scope on assignment, so you had the opposite problem when you wanted to reach a nested, non-global scope. Hence the nonlocal keyword added in py3k.
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.
> (function () { internalvariablewithnovardelcaration = "string"; })();
> internalvariablewithnovardeclaration
ReferenceError: internalvariablewithnovardeclaration is not defined
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)
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.
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
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.
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...
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. :)
> 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.
Global variables are basically instance variables on the process itself. They're going to be shared across all requests in all environments that handle more than a single request per process. If they had different semantics, they wouldn't really be what normal people call "global variables", would they?
So you just don't use them and everything's good. Which brings you to the real problem: JavaScript makes it distressingly easy to use a global variable where you meant to use a local one.
Seems like jesseedhillon was expecting that Node works like a browser: there's a lot of backend code in another language that handles the HTTP request, and then forks it over to a little JavaScript, and each request gets a new global scope.
It's not like that. There's a teeny bit of C-based backend code; it's the JavaScript part that controls alll the interesting stuff like opening sockets and parsing HTTP headers and the like to determine a request. In a single process, with a single global object, with lots of asynchronous I/O.
Node is JavaScript as a first-class programming language, not subservient to the DOM.
This has nothing to do with the DOM. There are existing server-side JS deployments that have no DOM, but do create separate globals per request.
There is nothing other than a conscious design decision that forces the Node code that you write to run against the same global as the code that handles the I/O guts in Node...
In particular, you could have the code that listens on the socket and chunks up the byte stream into HTTP requests running against one global while all further processing of those requests runs against a different global or set of globals. It's just that Node chose not to do that. But that's not an inherent limitation of either JS the language or the V8 implementation.
Well, that's what global variable means, shared among all functions in the process. Since there's only one process to handle all requests, they can access the global variables.
All languages used in web server that don't spawn a new process for each request have the same feature.
In JS, "global variable" means "property of the global object". No reference to "process".
It's trivial to have a single-process JS application with multiple JS global objects in it. Case in point: any web browser (and renderer processes don't change this: every iframe on a page has a separate global object).
It's just that Node _chose_ to reuse the same global object for multiple requests instead of using a clean execution environment.
I have to agree with jessedhillon: that sounds insane.
(I assume) the reason that node chose to go with a single global object (which is optional[0]) is that JS gets a bit hinky when dealing with multiple contexts: e.g., an array returned from a function defined in a different context will not satisfy `[] instanceof Array`, since the receiving module's instance of `Array` is not strictly equal to to called module's copy of `Array`.
In practice, this problem is why you see `Array.isArray`, `jQuery.isArray`, `_.isArray` and friends so often -- they exist to give a canonical, non-"check if any member of X's inheritance chain matches Y.prototype" way to determine if a given instance is actually an array.
So, in summation: yes, it's weird that Node has this, but ultimately it's a lesser-vilified problem of JavaScript's.
I have to amend my previous statement, these guys are right. I created this Python script here https://gist.github.com/1329943 and ran it. If global context were not shared, then each request would receive a new random number, but they don't. Each request gets the same number.
Even without doing this experiment, the result is obvious. There would have to be some meta process to start new interpreter processes here in order to prevent the same environment from being reused. Since that is not the case, because you have to invoke the python interpreter manually here, you cannot get multiple distinct results.
(Theoretically, in this code, you could get two different responses if they both entered the `if ... is None` clause in send_head() before one of the requests could cause the global variable to be set.)
You're running your whole server in python. So yes, there is only one process and its globals are very much global. Note that you are NOT writing some program that embeds the python interpreter and then runs it on python code of your choice.
In the case of Node, it's embedding V8. It can in fact run JS code of its choice in an environment of its choice, set up new globals, etc.
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.
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).
Indeed you can - but I suspect I know why Node has avoided this.
If you did take advantage of the runtime's support for multiple global objects, what would those global objects correspond to? The most natural scope would be "user session" - but if you go that route, now you support stateful user sessions. And before you know it you have developers storing loads of user session state in the server's RAM and wondering why they keep running out - and lose the ability to easily fail over to another server process.
So I imagine Node's author decided it was better to just keep things simple and discourage use of globals.
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.
Yes, I know JS in browsers is not multi-threaded. It runs multiple globals all on the same thread, as I said. Node is not multithreaded either; it could run a bunch of separate globals on the same thread as well, if it wanted to.
Well I don't think web browsers are really the same thing as server side code. Even individual pages in a browser are sandboxed from each other. Just like individual request/response cycles for a user session probably should be on the server (unless you go out of your way to break that, ie making some variable "static").
My point was that nothing inherent to JS forces sharing of a single global object across everything you do. The embedding (Node in this case) fully controls the granularity of such sharing.
"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/
You can do some poor man's load testing very simply with siege (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.
Melon Card.... I just signed up with a random email and it would allow me to remove the details for that email 'joe.doe@yourwebsite.com'... using your software as long as I did the captcha I could remove records for anyone's email as long as you can find them (the recommendation makes that easy)... you NEED to include verifying the email address before you can perform actions!!
Similar, but not quite as world-ending issue we had launching our Android app. Shortly (~36hrs) after launching, we had noticed that we were compiling with Cupcake (and we only intended to support Eclair+). We were pushing to have it out for SXSW; in a rush we just re-compiled, made sure it still launched, and pushed to the Market.
After everyone got some sleep, we realized that Cupcake didn't have multi-res support; and later phones quietly re-scaled everything to compensate. Compile to Eclair and well, things were a bit out of proportion.
I feel for you man, but did you not test with more than one user at the same time? I live in constant fear of this kind of thing, I always round up as many people as possible to test at once.
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.
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)
Is it just me or is this site completely unusable right now. I keep trying to say 'This isn't me!' or 'This is me!' with no results... maybe he has another missing 'var' somewhere?
Damn, I feel for ya; scope is hell sometimes, and js is all about giving you the power to shoot yourself in the foot. Interesting how bugs like this, while terrible and terribly easy to miss, tend to do a lot less damage on the front-end of things (or doesn't show up in a single thread/user environment). It was the potential of things like this happening that drove me towards languages like erlang and java on the server (even if js is still my favorite language).
> (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.
This is a major downside of interpreted languages, coupled with JS's diabolical scoping rules. Interpreted languages by their very nature, only reveal syntactic errors to you once you run them. I would like to see server side code written in compiled, statically typed languages, such as C/C++ to prevent such errors.
I hit a similar thing in some code I was writing, and ended up debugging it for about 5 hours. Couldn't figure out why when some tests ran, one of them would just die.
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.
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 :)
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.
As many have pointed out, there are absolutely tools that would catch this kind of problem in development. But the experience also speaks to the lack of sophisticated observability tools for Node (and just about every other popular dynamic language too).
This is a lesson we all have to learn. At least in Node it is easy to pick up with a tool like JSLint. In Java, you typically have some classes which have to be thread-safe (having no per-request state) and others which are instantiated per request. Developers have to be aware of the distinction, and to code accordingly. I fell into this trap when submitting a change to JBoss back in 2000; Rickard Oberg picked it up within hours (the diffs of all committed changes went out to everyone on the developer mailing list). I'm grateful to him for that, and I've done the same kind of review for many others since then.
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
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]"?
And that is what happens when you throw static type checking and the other safety features of compiled languages out the window, 'cause all the cool kids are doing it.
Maybe a language where variables are global by default and there are no concurrency semantics at all isn't so great for serving up web apps to concurrent users?
This reminds me of something I could not do. How do you load test a nodejs - socket.io installation? Is there a tool like apache bench to load test websockets?
I don't see this as a JavaScript specific problem. A similar issue can occur in IIS, for example. The details would be more elaborate, but the core issue would remain the same. A variable shared across requests screws something up under load. Seen it happen.
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.
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.
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.
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?
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.
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).
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.
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.
Congrats on launching. Saying this ruined your launch is a bit dramatic. The ability to put out fires when they happened is crucial to startups. Seems like the problem only existed for a few hours. It could have been worse. Write a test and move on
Oh wow. I've faced this exact same problem with missing `var`s more than once before. It's pretty ridiculous that JavaScript makes variables global by default.
Sorry to hear about your tragedy, hope it doesn't cause too many issues in the long run.
Reminds me of a time i had a similar bug.The more i tried the more i found it hard to discover the bug.One approach i usually use is to take a timeout and relax.But in your position there was no time for relaxing.
Node definitely needs better error handling, but what's nice about node is that each module is scoped in its own namespace, so if he had put each of his routes in its own module, that wouldn't have happened.
Ack, stuff likes this makes me very nervous about our production nodejs backend ... the error tracking is really quite difficult. But it's more a byproduct of javascript as opposed to node/express.
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.
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).
Exactly why we moved to CoffeeScript, without it you need to rigorously enforce coding conventions. This entails watching for missing vars, using jslint, having a script that searches for accidentally declared globals, only declaring vars at the top of a function, among other best practices. With CS you get all of that just by using it. If you can write high-standard code using a simpler, terser syntax, why avoid it?
> I 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.
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.
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.
"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 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.
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
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.
