src: Handle errors from andRecalculateMetadata

[dignifiedquire]

In outdated.js the errors from andRecalculateMetadata
were swallowed resulting in the cryptic errors seen in #9720.
This fixes this and handles the error so that the npm logs
the actual error.

[paulirish]

Thumbs up. This error message definitely pointed me to the fix.

Before and after:

[iarna]

Hi, just to be clear as nothing was said previously, but this needs a test.

[paulirish]

sounds good. @dignifiedquire can you hook it up?

[dignifiedquire]

Yes will take a look. @iarna For the future please leave a comment as gh does not send notifications for label changes and I had no idea this was reviewed :/

[dignifiedquire]

@iarna could you point me in a direction on how to test this best? I'm not sure how to make andRecalculateMetadata throw an error without injecting a mocked version of it, but I haven't seen any setup/packages for doing that in the tests so far, so my guess is you don't want to go this route during testing. But I'm afraid I need some pointers on how to make it fail at this specific point then.

[iarna]

@dignifiedquire Sure…

So andRecalculateMetadata is just a tiny wrapper around recalculateMetadata– we could inject that, but I think maybe injecting readPackageTree would be better, as andRecalculateMetadata passes on the error if readPackageTree produced one. Regardless, we use require-inject to do our dep injection for tests.

In a test that might look like this:

var test = require('tap').test
var npm = require('../../lib/npm') // the ../../ is because tests live in test/tap
var requireInject = require('require-inject')

test('setup', function (t) {
  // because of unfortunate early architecture decisions we can't require any module
  // until `npm.load` has completed.
  npm.load({},t.end)
})

test('outdated', function (t) {
    var outdated = requireInject('../../lib/outdated.js', {
        'read-package-tree': function (_, next) {
            next(new Error('breaking on purpose'))
        }
    })
    outdated(…)
})

[dignifiedquire]

Thanks @iarna will try and get the test done tomorrow

[iarna]

We're closing this pull request as it has gone sixty days without activity. In our experience pull requests that go this long are unlikely to land. If you have updates to it, please feel free to open a new pull request.

For more information about our new issue aging policies and why we've instituted them please see our blog post.