Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

error-message: strip version info from pkg on E404 #132

Merged
merged 4 commits into from
Feb 18, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/utils/error-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,12 @@ function errorMessage (er) {
var msg = er.message.replace(/^404\s+/, '')
short.push(['404', msg])
if (er.pkgid && er.pkgid !== '-') {
var pkg = er.pkgid.replace(/(?!^)@.*$/, '')

detail.push(['404', ''])
detail.push(['404', '', "'" + er.pkgid + "' is not in the npm registry."])
detail.push(['404', '', "'" + pkg + "' is not in the npm registry."])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output should still include the version, because if I say npm i npm@20.0.0 I don't want an error saying that npm isn't in the registry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems I forgot to address this earlier, but I added a fix (not sure why I changed it in the first place).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing this makes me want to use template literals. =)


var valResult = nameValidator(er.pkgid)
var valResult = nameValidator(pkg)

if (valResult.validForNewPackages) {
detail.push(['404', 'You should bug the author to publish it (or use the name yourself!)'])
Expand Down
72 changes: 72 additions & 0 deletions test/tap/404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
'use strict'
const path = require('path')
const test = require('tap').test
const Tacks = require('tacks')
const File = Tacks.File
const Dir = Tacks.Dir
const common = require('../common-tap.js')

const e404 = /test-npm-404' is not in the npm registry/
const invalidPackage = /Your package name is not valid, because[\s\S]+1\. name can only contain URL-friendly characters/

const basedir = path.join(__dirname, path.basename(__filename, '.js'))
const testdir = path.join(basedir, 'testdir')
const cachedir = path.join(basedir, 'cache')
const globaldir = path.join(basedir, 'global')
const tmpdir = path.join(basedir, 'tmp')

const env = common.newEnv().extend({
npm_config_cache: cachedir,
npm_config_tmp: tmpdir,
npm_config_prefix: globaldir,
npm_config_registry: common.registry,
npm_config_loglevel: 'error'
})

const fixture = new Tacks(Dir({
cache: Dir(),
global: Dir(),
tmp: Dir(),
testdir: Dir({
'package.json': File({
name: 'test',
version: '1.0.0'
})
})
}))

function setup () {
cleanup()
fixture.create(basedir)
}

function cleanup () {
fixture.remove(basedir)
}

test('setup', function (t) {
setup()
return common.fakeRegistry.listen()
})

test('404 message for basic package', function (t) {
return common.npm(['install', 'test-npm-404'], {cwd: testdir, env}).then(([code, stdout, stderr]) => {
t.is(code, 1, 'error code')
t.match(stderr, e404, 'error output')
t.notMatch(stderr, invalidPackage, 'no invalidity error')
})
})

test('404 message for scoped package', function (t) {
return common.npm(['install', '@npm/test-npm-404'], {cwd: testdir, env}).then(([code, stdout, stderr]) => {
t.is(code, 1, 'error code')
t.match(stderr, e404, 'error output')
t.notMatch(stderr, invalidPackage, 'no invalidity error')
})
})

test('cleanup', function (t) {
common.fakeRegistry.close()
cleanup()
t.done()
})