Ensure that stream failures are sent up the promise chain

[lddubeau]

The problematic code prior to this PR is this:

function tryExtract (spec, tarStream, dest, opts) {
  return new BB((resolve, reject) => {
    tarStream.on('error', reject)
    setImmediate(resolve)
  })
    .then(() => rimraf(dest))
    .then(() => mkdirp(dest))
    .then(() => new BB((resolve, reject) => {
      const xtractor = extractStream(spec, dest, opts)
      tarStream.on('error', reject)
      xtractor.on('error', reject)
      xtractor.on('close', resolve)
      tarStream.pipe(xtractor)
    }))
    .catch(err => {
      if (err.code === 'EINTEGRITY') {
        err.message = `Verification failed while extracting ${spec}:\n${err.message}`
      }
      throw err
    })
}

There's a race condition in this code. The problem is that neither of the tarStream.on('error', reject) lines are capable of reporting all errors with tarStream. If a tarStream error happens prior to the outer resolve being called, then the first tarStream.on('error'... will pass the error up correctly. (Actually, I don't think this case is possible. At some point I had instrumented the callback passed to both calls to tarStream.on('error'... and the callback passed to setImmediate(...), and it never ever ever ever ever happened that the first tarStream.on('error' caught an error prior to the call scheduled with setImmediate.)

However, for those errors that cannot be detected immediately the first tarStream.on('error' call is useless. By the time these errors are raised, the resolve call scheduled by setImmediate will have happened, and thus even if the first tarStream.on('error' calls reject, this will have no effect whatsoever because the promise has already resolved.

So there is a window of time between the first tarStream.on('error' and the second one where if a tarStream error occurs, it won't be sent up the promise chain. How big a window it is depends on how long rimraf and mkdirp take to do their work. If a tarStream error is raised during that window, the first error event handler cannot do anything about it, and the 2nd event handler is not setup yet so it cannot do anything either.

The test I created to test the issue changes PATH to an empty string temporarily so that executing git will fail. It replicates the problem I initially ran into: I was running npm in a docker build where git was not installed, and I got the utterly non-descriptive cb() never called error message. (And nothing else. I've seen cases where cb() never called is accompanied with some more information but in my case that was the sole error message and --loglevel=silly did not help either.)

[zkat]

Hi! Thanks for the PR! Three things:

1. I see this isn't passing on AppVeyor because the test is not completing.
2. Please file this PR over at https://github.com/npm/pacote, which is the new home for this repository (sorry for the confusion, I'm working on shuffling things around over to the npm org).
3. This is a really excellent, detailed description of the PR! I appreciate another set of eyes looking at the weird race conditions inherent in mixing streams and Promises. It's been a huge source of frustration, but it'd be awesome if this turns out to be the cb() never called that a lot of folks have been getting. Thank you!

I'm going to close this specific PR because I'm about to lock down the repo for archival purposes, but I'll see you over at npm/pacote, I hope!