How One Missing `var` Ruined our Launch
blog.meloncard.com
blog.meloncard.com
/* "use strict" */
$ node
> "use strict"; initial = "foo";
ReferenceError: initial is not defined
at repl:1:17
at Interface.<anonymous> (repl.js:168:22)
...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.
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.
The string literal doesn't make anyone _happy_, but it was most compatible with the existing widely deployed toolchains, as I understand.
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.
use v5.12;
((x)-> x = 2; arguments[0] is 1)(1) # true
((x)-> arguments[0] = 2; x is 1)(1) # true
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.
for (x=0;x<variable.length;x++) {
// blah
}
edit: initialise/declare brain freeze
Do you mean s/initialising/declaring/, or am I just really confused?
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.
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.
My eyebrow just raised so far my forehead cramped.
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.
Not to beat on you, but didn't you discover the error after users started using it in a big news day?
It also doesn't happen in languages where you must say self.x to access a property/field called x.
Only it is worse in Python because, lacking `var` keyword, you will shadow the variable in Python
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.
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 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.
jshint would have caught it. You need to run jshint on your code or you will get silly errors like this. Simple.
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.
Note that nowadays going to Coffeescript from Javascript is quite easy: http://js2coffee.org/
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.
->
outerVar = "outer"
->
# this will output "undefined"
# as CoffeeScript has redeclared it for the loop:
console.log outerVar
0 for outerVar in [0]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.
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.
"use strict" is catching a lot more errors, though frustratingly enough, not until runtime.
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.
Scheme got it right sometime in the 1970's.
(let ((x initial-value)) ; binding
(set! x new-value)) ; assignment
let x = initial-value ;
x := new-value ;https://en.wikipedia.org/wiki/ALGOL_58#ALGOL_58.27s_influenc...
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?
sum = 0
for v in someList:
smu += v
print sum # prints 0, doh!Can someone please implement strict mode for Node?
It's already implemented, you just have to enable it.
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
NameError: name 'smu' is not defined >>> for v in (1,2,3):
... smu+=v
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
NameError: name 'smu' is not defined (define x 42) ;; global
(let ((x 42)) ;; local, only defined inside the enclosing parenthesis.
(+ x x))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.
Get a better editor, and JSHint, and "use strict";
Implicitly declared globals can be statically checked, there is no reason not to.
> (function () { internalvariablewithnovardelcaration = "string"; })();
> internalvariablewithnovardeclaration
ReferenceError: internalvariablewithnovardeclaration is not definedhttps://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)
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 problemsThis 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.
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>
If so, that is an insane design.
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.
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).
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.
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.
But doing global per request would not be that complicated, if desired.
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.
> Global variables are basically instance variables on the
> process itself.
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.
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.
All languages used in web server that don't spawn a new process for each request have the same feature.
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.
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.)
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.
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.
[0]: https://github.com/joyent/node/blob/master/lib/module.js#L51 -- `NODE_MODULE_CONTEXTS=1 node <file.js>` will run separate modules in separate global contexts.
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/
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.
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.
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)
> (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'
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.
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.
var a = "foo",
b = "bar",
c = "baz";
var
a = "foo"
, b = "bar"
, c = "baz"
; var a = "foo";
var b = "bar";
var c = "baz";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.
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?
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.
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 globallylet $JS_CMD='node'
Sorry to hear about your tragedy, hope it doesn't cause too many issues in the long run.
bullshit.
jslint or clojure compiler would have found this error at "compile time."
coffeescript would have made it a non-issue.
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).
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.
Sorry, that's incorrect. If you cannot fully simulate your environment for purposes of validation, you're not covering your bases.
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.
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.
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. :)
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.
Perfect example of a developer hating JS just because he/she didn't bother to learn it first and got burned.