deps: upgrade npm to 2.3.0

[othiym23]

As discussed on #9078, here is npm@2.3.0. make test-npm ran without issue on OS X Yosemite.

r=@misterdjules

[misterdjules]

@othiym23 Thank you! I'll take a look as soon as possible.

[misterdjules]

Running npm's tests suite with current node v0.12 makes a lot of tests time out. Running the same tests suite with node v0.10.36 works fine.

It seems that the problem is that the npm-registry-mock server still has active connections after it's closed. For instance, ./deps/npm/test/tap/uninstall-package.js hangs with the following output when NODE_DEBUG=net:

➜  v0.12 git:(v0.12) ✗ NODE_DEBUG=net ./node ./deps/npm/test/tap/uninstall-package.js
NET 12638: listen2 null 1337 4 false
NET 12638: _listen2: create a handle
NET 12638: bind to anycast
NET 12638: createConnection [ { domain: null,
    _events: 
     { error: [Object],
       complete: [Function],
       pipe: [Function],
       socket: [Function] },
    _maxListeners: undefined,
    uri: 
     { protocol: 'http:',
       slashes: true,
       auth: null,
       host: 'localhost:1337',
       port: '1337',
       hostname: 'localhost',
       hash: null,
       search: null,
       query: null,
       pathname: '/underscore',
       path: '/underscore',
       href: 'http://localhost:1337/underscore' },
    callback: [Function],
    method: 'GET',
    headers: 
     { 'accept-encoding': 'gzip',
       version: '2.3.0',
       accept: 'application/json',
       referer: 'install .',
       'npm-session': 'eb084e5eba662208',
       'user-agent': 'npm/2.3.0 node/v0.11.16-pre darwin x64',
       host: 'localhost:1337' },
    localAddress: undefined,
    strictSSL: true,
    cert: null,
    key: null,
    ca: null,
    agent: 
     { domain: null,
       _events: [Object],
       _maxListeners: undefined,
       defaultPort: 80,
       protocol: 'http:',
       options: [Object],
       requests: {},
       sockets: [Object],
       freeSockets: {},
       keepAliveMsecs: 1000,
       keepAlive: true,
       maxSockets: Infinity,
       maxFreeSockets: 256 },
    followRedirect: true,
    encoding: null,
    readable: true,
    writable: true,
    explicitMethod: true,
    canTunnel: false,
    setHeader: [Function],
    hasHeader: [Function],
    getHeader: [Function],
    removeHeader: [Function],
    qsLib: { stringify: [Function], parse: [Function] },
    pool: {},
    dests: [],
    __isRequestRequest: true,
    _callback: [Function],
    proxy: null,
    tunnel: false,
    _redirectsFollowed: 0,
    maxRedirects: 10,
    allowRedirect: [Function],
    followRedirects: true,
    followAllRedirects: false,
    redirects: [],
    setHost: true,
    originalCookieHeader: undefined,
    _disableCookies: true,
    _jar: undefined,
    port: '1337',
    host: 'localhost',
    path: null,
    httpModule: 
     { IncomingMessage: [Object],
       METHODS: [Object],
       OutgoingMessage: [Object],
       ServerResponse: [Object],
       STATUS_CODES: [Object],
       Agent: [Object],
       globalAgent: [Object],
       ClientRequest: [Object],
       request: [Function],
       get: [Function],
       _connectionListener: [Function: connectionListener],
       Server: [Object],
       createServer: [Function],
       Client: [Function: deprecated],
       createClient: [Function: deprecated] },
    _started: true,
    href: 'http://localhost:1337/underscore',
    keepAlive: true,
    servername: 'localhost' } ]
NET 12638: pipe false null
NET 12638: connect: find host localhost
NET 12638: connect: dns options [object Object]
NET 12638: _read
NET 12638: _read wait for connection
NET 12638: createConnection [ { domain: null,
    _events: 
     { error: [Object],
       complete: [Function],
       pipe: [Function],
       socket: [Function] },
    _maxListeners: undefined,
    uri: 
     { protocol: 'http:',
       slashes: true,
       auth: null,
       host: 'localhost:1337',
       port: '1337',
       hostname: 'localhost',
       hash: null,
       search: null,
       query: null,
       pathname: '/request',
       path: '/request',
       href: 'http://localhost:1337/request' },
    callback: [Function],
    method: 'GET',
    headers: 
     { 'accept-encoding': 'gzip',
       version: '2.3.0',
       accept: 'application/json',
       referer: 'install .',
       'npm-session': 'eb084e5eba662208',
       'user-agent': 'npm/2.3.0 node/v0.11.16-pre darwin x64',
       host: 'localhost:1337' },
    localAddress: undefined,
    strictSSL: true,
    cert: null,
    key: null,
    ca: null,
    agent: 
     { domain: null,
       _events: [Object],
       _maxListeners: undefined,
       defaultPort: 80,
       protocol: 'http:',
       options: [Object],
       requests: {},
       sockets: [Object],
       freeSockets: {},
       keepAliveMsecs: 1000,
       keepAlive: true,
       maxSockets: Infinity,
       maxFreeSockets: 256 },
    followRedirect: true,
    encoding: null,
    readable: true,
    writable: true,
    explicitMethod: true,
    canTunnel: false,
    setHeader: [Function],
    hasHeader: [Function],
    getHeader: [Function],
    removeHeader: [Function],
    qsLib: { stringify: [Function], parse: [Function] },
    pool: {},
    dests: [],
    __isRequestRequest: true,
    _callback: [Function],
    proxy: null,
    tunnel: false,
    _redirectsFollowed: 0,
    maxRedirects: 10,
    allowRedirect: [Function],
    followRedirects: true,
    followAllRedirects: false,
    redirects: [],
    setHost: true,
    originalCookieHeader: undefined,
    _disableCookies: true,
    _jar: undefined,
    port: '1337',
    host: 'localhost',
    path: null,
    httpModule: 
     { IncomingMessage: [Object],
       METHODS: [Object],
       OutgoingMessage: [Object],
       ServerResponse: [Object],
       STATUS_CODES: [Object],
       Agent: [Object],
       globalAgent: [Object],
       ClientRequest: [Object],
       request: [Function],
       get: [Function],
       _connectionListener: [Function: connectionListener],
       Server: [Object],
       createServer: [Function],
       Client: [Function: deprecated],
       createClient: [Function: deprecated] },
    _started: true,
    href: 'http://localhost:1337/request',
    keepAlive: true,
    servername: 'localhost' } ]
NET 12638: pipe false null
NET 12638: connect: find host localhost
NET 12638: connect: dns options [object Object]
NET 12638: _read
NET 12638: _read wait for connection
NET 12638: onconnection
NET 12638: _read
NET 12638: Socket._read readStart
NET 12638: afterConnect
NET 12638: _read
NET 12638: Socket._read readStart
NET 12638: onconnection
NET 12638: _read
NET 12638: Socket._read readStart
NET 12638: afterConnect
NET 12638: _read
NET 12638: Socket._read readStart
NET 12638: onread 242
NET 12638: got data
NET 12638: _read
NET 12638: afterWrite 0
NET 12638: afterWrite call cb
NET 12638: onread 41061
NET 12638: got data
NET 12638: _read
NET 12638: onread 239
NET 12638: got data
NET 12638: _read
NET 12638: onread 65536
NET 12638: got data
NET 12638: _read
NET 12638: onread 65536
NET 12638: got data
NET 12638: _read
NET 12638: onread 56876
NET 12638: got data
NET 12638: _read
NET 12638: afterWrite 0
NET 12638: afterWrite call cb
NET 12638: onread 65536
NET 12638: got data
NET 12638: _read
NET 12638: onread 65536
NET 12638: got data
NET 12638: _read
NET 12638: onread 65536
NET 12638: got data
NET 12638: _read
NET 12638: onread 30955
NET 12638: got data
NET 12638: _read
NET 12638: onread 279
NET 12638: got data
NET 12638: _read
NET 12638: afterWrite 0
NET 12638: afterWrite call cb
NET 12638: onread 273
NET 12638: got data
NET 12638: _read
NET 12638: onread 58813
NET 12638: got data
NET 12638: _read
NET 12638: afterWrite 0
NET 12638: afterWrite call cb
NET 12638: onread 4006
NET 12638: got data
NET 12638: _read
request@0.9.5 node_modules/request

underscore@1.3.3 node_modules/underscore
unbuild underscore@1.3.3
unbuild request@0.9.5
NET 12638: SERVER _emitCloseIfDrained
NET 12638: SERVER handle? false   connections? 2
TAP version 13
# returns a list of removed items
ok 1 should be equivalent
# cleanup

1..1
# tests 1
# pass  1

# ok
^C
➜  v0.12 git:(v0.12) ✗

[misterdjules]

The breaking change seems to be npm/npm@c2cccd4, and more specifically the use of keepAlive: true for the http agent.

I'm a bit puzzled by the commit message that states:

• ensure that Keep-Alive requests keep working properly with Node 0.11 /
  io.js 1.0.1

because this change actually seems to break node v0.11.x and io.js 1.0.1.

@othiym23 Could you please provide me with more context on why this change was needed?

[othiym23]

See npm/npm-registry-client@4701a29 for @fengmk2's commit. Maybe he can provide you with more detail?

I'm still confused as to why you are seeing these test failures but I'm not. Have you rebased my change into a later base?

[misterdjules]

@othiym23 It seems that the test-all npm script uses the node binary in the PATH. Is it possible that the node binary in your PATH when you run tests is at a different version that doesn't expose this issue?

Also, when testing, I rebased this PR on top of the current tip of v0.12.

[othiym23]

It seems that the test-all npm script uses the node binary in the PATH.

How do you figure? The 'Makefile' is using the freshly-built node to run the tests. I will rebase against v0.12 HEAD, but as far as I can tell, I'm using the newly-built Node.

[othiym23]

And test-all uses process.execPath in child scripts, as do all npm lifecycle scripts.

[othiym23]

Or, rather, I thought it did. I applied this patch to the Makefile:

diff --git a/Makefile b/Makefile
index 6c148eb..9f388c4 100644
--- a/Makefile
+++ b/Makefile
@@ -154,12 +154,15 @@ test-npm: node
        cd deps/npm ; npm_config_cache="$(shell pwd)/npm-cache" \
             npm_config_prefix="$(shell pwd)/npm-prefix" \
             npm_config_tmp="$(shell pwd)/npm-tmp" \
-            ../../node cli.js install
+            PATH="../..:/bin:/usr/bin" \
+            node cli.js install
        cd deps/npm ; npm_config_cache="$(shell pwd)/npm-cache" \
             npm_config_prefix="$(shell pwd)/npm-prefix" \
             npm_config_tmp="$(shell pwd)/npm-tmp" \
-            ../../node cli.js run-script test-all && \
-            ../../node cli.js prune --prod && \
+            PATH="../..:/bin:/usr/bin" \
+            node cli.js run-script test-all && \
+            PATH="../..:/bin:/usr/bin" \
+            node cli.js prune --prod && \
             cd ../.. && \
             rm -rf npm-cache npm-tmp npm-prefix

...and now I am seeing some failures as well. I'm reluctant to back out @fengmk2's changes without a better understanding of what the changes were meant to accomplish -- replacing the agent hasn't caused any problems (that I know of) in iojs or Node 0.10, so there's something happening at a lower level that I don't understand.

[misterdjules]

The problem can be reproduced outside of npm tests. I'll investigate further and I'll keep you posted. Thank you!

[misterdjules]

Thanks to @tjfontaine, we've been able to find out the issue with the tests timing out. Basically, when tests "npm install" a package, the npm-registry-client uses a HTTP agent that with keepAlive enabled.

On both sides, the connection is not closed by default when the HTTP request is finished because the client explicitly asked for the connection to be kept alive.

On the client side, the tcp connection handle is unref'd and thus doesn't hold the loop active. However, on the server side, the tcp handle is not unref'd and thus holds the loop active, and eventually makes node hangs, which in turn makes tests time out.

It can be verified for instance by unref'ing the connection on the server's side.

This is expected behavior that is not intuitive when both the client and the server are in the same process.

Indeed, most of the time, the client and the server are not the same process and the code that hangs works as expected intuitively.

If this test had the client and the server in separate processes, when the client tcp handle is unref'd, the client process would exit and this would close the connection, unref'ing the tcp handle in the server process, which would then also exit properly.

The tests will need to be fixed in npm, but in the meantime this PR is LGTM, as binary packages have been tested on OS X and Windows without any issue, and the only tests failing fail by timing out due to this keepAlive issue, but the actual tests pass.

[tjfontaine]

I concur -- LGTM

[misterdjules]

@othiym23 Thank you, landed in 491ac6a!

[misterdjules]

Also created #9107 to avoid running npm's tests with a different version of node than intended when running 'make test-npm'.