Skip to content

Commit

Permalink
open: handle missing open bin error (#20582)
Browse files Browse the repository at this point in the history
PR-URL: npm/npm#20582
Credit: @bcoe
Reviewed-By: @zkat
  • Loading branch information
bcoe authored and zkat committed May 11, 2018
1 parent 1854b1c commit eb75220
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 38 deletions.
4 changes: 2 additions & 2 deletions lib/auth/legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ const npm = require('../npm.js')
const output = require('../utils/output.js')
const pacoteOpts = require('../config/pacote')
const fetchOpts = require('../config/fetch-opts')
const opener = require('opener')
const openUrl = require('../utils/open-url')

const openerPromise = (url) => new Promise((resolve, reject) => {
opener(url, { command: npm.config.get('browser') }, (er) => er ? reject(er) : resolve())
openUrl(url, 'to complete your login please visit', (er) => er ? reject(er) : resolve())
})

const loginPrompter = (creds) => {
Expand Down
7 changes: 2 additions & 5 deletions lib/auth/sso.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var log = require('npmlog')
var npm = require('../npm.js')
var output = require('../utils/output')
var opener = require('opener')
var openUrl = require('../utils/open-url')

module.exports.login = function login (creds, registry, scope, cb) {
var ssoType = npm.config.get('sso-type')
Expand All @@ -22,10 +22,7 @@ module.exports.login = function login (creds, registry, scope, cb) {
if (!doc || !doc.token) return cb(new Error('no SSO token returned'))
if (!doc.sso) return cb(new Error('no SSO URL returned by services'))

output('If your browser doesn\'t open, visit ' +
doc.sso +
' to complete authentication')
opener(doc.sso, { command: npm.config.get('browser') }, function () {
openUrl(doc.sso, 'to complete your login please visit', function () {
pollForSession(registry, doc.token, function (err, username) {
if (err) return cb(err)

Expand Down
5 changes: 2 additions & 3 deletions lib/bugs.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
module.exports = bugs

var npm = require('./npm.js')
var log = require('npmlog')
var opener = require('opener')
var openUrl = require('./utils/open-url')
var fetchPackageMetadata = require('./fetch-package-metadata.js')
var usage = require('./utils/usage')

Expand All @@ -27,6 +26,6 @@ function bugs (args, cb) {
url = 'https://www.npmjs.org/package/' + d.name
}
log.silly('bugs', 'url', url)
opener(url, { command: npm.config.get('browser') }, cb)
openUrl(url, 'bug list available at the following URL', cb)
})
}
5 changes: 2 additions & 3 deletions lib/docs.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module.exports = docs

var npm = require('./npm.js')
var opener = require('opener')
var openUrl = require('./utils/open-url')
var log = require('npmlog')
var fetchPackageMetadata = require('./fetch-package-metadata.js')
var usage = require('./utils/usage')
Expand Down Expand Up @@ -37,6 +36,6 @@ function getDoc (project, cb) {
if (er) return cb(er)
var url = d.homepage
if (!url) url = 'https://www.npmjs.org/package/' + d.name
return opener(url, {command: npm.config.get('browser')}, cb)
return openUrl(url, 'docs available at the following URL', cb)
})
}
4 changes: 2 additions & 2 deletions lib/help.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ var path = require('path')
var spawn = require('./utils/spawn')
var npm = require('./npm.js')
var log = require('npmlog')
var opener = require('opener')
var openUrl = require('./utils/open-url')
var glob = require('glob')
var didYouMean = require('./utils/did-you-mean')
var cmdList = require('./config/cmd-list').cmdList
Expand Down Expand Up @@ -127,7 +127,7 @@ function viewMan (man, cb) {
break

case 'browser':
opener(htmlMan(man), { command: npm.config.get('browser') }, cb)
openUrl(htmlMan(man), 'help available at the following URL', cb)
break

default:
Expand Down
5 changes: 2 additions & 3 deletions lib/repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ module.exports = repo

repo.usage = 'npm repo [<pkg>]'

var npm = require('./npm.js')
var opener = require('opener')
var openUrl = require('./utils/open-url')
var hostedGitInfo = require('hosted-git-info')
var url_ = require('url')
var fetchPackageMetadata = require('./fetch-package-metadata.js')
Expand Down Expand Up @@ -32,7 +31,7 @@ function getUrlAndOpen (d, cb) {

if (!url) return cb(new Error('no repository: could not get url'))

opener(url, { command: npm.config.get('browser') }, cb)
openUrl(url, 'repository available at the following URL', cb)
}

function unknownHostedUrl (url) {
Expand Down
16 changes: 16 additions & 0 deletions lib/utils/open-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict'
const npm = require('../npm.js')
const output = require('./output.js')
const opener = require('opener')

// attempt to open URL in web-browser, print address otherwise:
module.exports = function open (url, errMsg, cb, browser = npm.config.get('browser')) {
opener(url, { command: npm.config.get('browser') }, (er) => {
if (er && er.code === 'ENOENT') {
output(`${errMsg}:\n\n${url}`)
return cb()
} else {
return cb(er)
}
})
}
14 changes: 4 additions & 10 deletions test/tap/adduser-oauth.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test('npm login', function (t) {
s.get(
'/-/whoami', { authorization: 'Bearer foo' }
).max(1).reply(401, {})
var runner = common.npm(
common.npm(
[
'login',
'--registry', common.registry,
Expand All @@ -69,15 +69,9 @@ test('npm login', function (t) {
}
)

var buf = ''
runner.stdout.on('data', function (chunk) {
buf += chunk.toString('utf8')
if (buf.match(/complete authentication/)) {
s.get(
'/-/whoami', { authorization: 'Bearer foo' }
).reply(200, { username: 'igotauthed' })
}
})
s.get(
'/-/whoami', { authorization: 'Bearer foo' }
).reply(200, { username: 'igotauthed' })
})
})

Expand Down
14 changes: 4 additions & 10 deletions test/tap/adduser-saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test('npm login', function (t) {
s.get(
'/-/whoami', { authorization: 'Bearer foo' }
).max(1).reply(401, {})
var runner = common.npm(
common.npm(
[
'login',
'--registry', common.registry,
Expand All @@ -69,15 +69,9 @@ test('npm login', function (t) {
}
)

var buf = ''
runner.stdout.on('data', function (chunk) {
buf += chunk.toString('utf8')
if (buf.match(/complete authentication/)) {
s.get(
'/-/whoami', { authorization: 'Bearer foo' }
).reply(200, { username: 'igotauthed' })
}
})
s.get(
'/-/whoami', { authorization: 'Bearer foo' }
).reply(200, { username: 'igotauthed' })
})
})

Expand Down

0 comments on commit eb75220

Please sign in to comment.