Throw custom errors from spawn (#32)

Ref: npm/cli#3149

Author: lukekarrys

Diff for: lib/errors.js

@@ -0,0 +1,36 @@
+
+const maxRetry = 3
+
+class GitError extends Error {
+  shouldRetry () {
+    return false
+  }
+}
+
+class GitConnectionError extends GitError {
+  constructor (message) {
+    super('A git connection error occurred')
+  }
+
+  shouldRetry (number) {
+    return number < maxRetry
+  }
+}
+
+class GitPathspecError extends GitError {
+  constructor (message) {
+    super('The git reference could not be found')
+  }
+}
+
+class GitUnknownError extends GitError {
+  constructor (message) {
+    super('An unknown git error occurred')
+  }
+}
+
+module.exports = {
+  GitConnectionError,
+  GitPathspecError,
+  GitUnknownError
+}

Diff for: lib/index.js

@@ -4,5 +4,6 @@ module.exports = {
   spawn: require('./spawn.js'),
   is: require('./is.js'),
   find: require('./find.js'),
-  isClean: require('./is-clean.js')
+  isClean: require('./is-clean.js'),
+  errors: require('./errors.js')
 }

Diff for: lib/make-error.js

@@ -0,0 +1,33 @@
+const {
+  GitConnectionError,
+  GitPathspecError,
+  GitUnknownError
+} = require('./errors.js')
+
+const connectionErrorRe = new RegExp([
+  'remote error: Internal Server Error',
+  'The remote end hung up unexpectedly',
+  'Connection timed out',
+  'Operation timed out',
+  'Failed to connect to .* Timed out',
+  'Connection reset by peer',
+  'SSL_ERROR_SYSCALL',
+  'The requested URL returned error: 503'
+].join('|'))
+
+const missingPathspecRe = /pathspec .* did not match any file\(s\) known to git/
+
+function makeError (er) {
+  const message = er.stderr
+  let gitEr
+  if (connectionErrorRe.test(message)) {
+    gitEr = new GitConnectionError(message)
+  } else if (missingPathspecRe.test(message)) {
+    gitEr = new GitPathspecError(message)
+  } else {
+    gitEr = new GitUnknownError(message)
+  }
+  return Object.assign(gitEr, er)
+}
+
+module.exports = makeError

Diff for: lib/should-retry.js

@@ -1,6 +1,6 @@
 const spawn = require('@npmcli/promise-spawn')
 const promiseRetry = require('promise-retry')
-const shouldRetry = require('./should-retry.js')
+const makeError = require('./make-error.js')
 const whichGit = require('./which.js')
 const makeOpts = require('./opts.js')
 const procLog = require('./proc-log.js')
@@ -33,10 +33,11 @@ module.exports = (gitArgs, opts = {}) => {
 
     return spawn(gitPath, args, makeOpts(opts))
       .catch(er => {
-        if (!shouldRetry(er.stderr, number)) {
-          throw er
+        const gitError = makeError(er)
+        if (!gitError.shouldRetry(number)) {
+          throw gitError
         }
-        retry(er)
+        retry(gitError)
       })
   }, retry)
 }

Diff for: lib/spawn.js

@@ -3,5 +3,10 @@ const t = require('tap')
 t.match(git, {
   clone: Function,
   revs: Function,
-  spawn: Function
+  spawn: Function,
+  errors: {
+    GitConnectionError: /GitConnectionError/,
+    GitPathspecError: /GitPathspecError/,
+    GitUnknownError: /GitUnknownError/
+  }
 })

Diff for: test/index.js

@@ -0,0 +1,30 @@
+const _makeError = require('../lib/make-error.js')
+const errors = require('../lib/errors.js')
+const t = require('tap')
+
+const makeError = (message) =>
+  // Create the error with properties like it came from promise-spawn
+  _makeError(Object.assign(new Error(), { stderr: message }))
+
+t.test('throw matching error for missing pathspec', (t) => {
+  const missingPathspec = makeError('error: pathspec \'foo\' did not match any file(s) known to git')
+  t.ok(missingPathspec instanceof errors.GitPathspecError, 'error is a pathspec error')
+
+  t.end()
+})
+
+t.test('only transient connection errors are retried', (t) => {
+  const sslError = makeError('SSL_ERROR_SYSCALL')
+  t.ok(sslError.shouldRetry(1), 'transient error, not beyond max')
+  t.ok(sslError instanceof errors.GitConnectionError, 'error is a connection error')
+
+  const unknownError = makeError('asdf')
+  t.notOk(unknownError.shouldRetry(1), 'unknown error, do not retry')
+  t.ok(unknownError instanceof errors.GitUnknownError, 'error is an unknown error')
+
+  const connectError = makeError('Failed to connect to fooblz Timed out')
+  t.notOk(connectError.shouldRetry(69), 'beyond max retries, do not retry')
+  t.ok(connectError instanceof errors.GitConnectionError, 'error is a connection error')
+
+  t.end()
+})

Diff for: test/make-error.js

@@ -1,5 +1,6 @@
 const spawn = require('../lib/spawn.js')
 const procLog = require('../lib/proc-log.js')
+const errors = require('../lib/errors.js')
 
 const t = require('tap')
 t.rejects(spawn(['status'], { git: false }), {
@@ -44,9 +45,10 @@ t.test('argument test for allowReplace', async t => {
 t.test('retries', t => {
   const logs = []
   process.on('log', (...log) => logs.push(log))
+  const gitMessage = 'Connection timed out\n'
   const te = resolve(repo, 'transient-error.js')
   fs.writeFileSync(te, `
-console.error('Connection timed out')
+console.error('${gitMessage.trim()}')
 process.exit(1)
   `)
   const retryOptions = {
@@ -65,15 +67,15 @@ process.exit(1)
       fetchRetryMintimeout: 1
     }
   }
-  const er = {
-    message: 'command failed',
+  const er = Object.assign(new errors.GitConnectionError(gitMessage), {
     cmd: process.execPath,
     args: [te],
     code: 1,
     signal: null,
     stdout: '',
-    stderr: 'Connection timed out\n'
-  }
+    stderr: gitMessage,
+    message: 'A git connection error occurred'
+  })
   Object.keys(retryOptions).forEach(n => t.test(n, t =>
     t.rejects(spawn([te], {
       cwd: repo,
@@ -98,3 +100,53 @@ process.exit(1)
     })))
   t.end()
 })
+
+t.test('missing pathspec', t => {
+  const gitMessage = 'error: pathspec \'foo\' did not match any file(s) known to git\n'
+  const te = resolve(repo, 'pathspec-error.js')
+  fs.writeFileSync(te, `
+console.error("${gitMessage.trim()}")
+process.exit(1)
+  `)
+  const er = Object.assign(new errors.GitPathspecError(gitMessage), {
+    cmd: process.execPath,
+    args: [te],
+    code: 1,
+    signal: null,
+    stdout: '',
+    stderr: gitMessage,
+    message: 'The git reference could not be found'
+  })
+  t.rejects(spawn([te], {
+    cwd: repo,
+    git: process.execPath,
+    allowReplace: true,
+    log: procLog
+  }), er)
+  t.end()
+})
+
+t.test('unknown git error', t => {
+  const gitMessage = 'error: something really bad happened to git\n'
+  const te = resolve(repo, 'unknown-error.js')
+  fs.writeFileSync(te, `
+console.error("${gitMessage.trim()}")
+process.exit(1)
+  `)
+  const er = Object.assign(new errors.GitUnknownError(gitMessage), {
+    cmd: process.execPath,
+    args: [te],
+    code: 1,
+    signal: null,
+    stdout: '',
+    stderr: gitMessage,
+    message: 'An unknown git error occurred'
+  })
+  t.rejects(spawn([te], {
+    cwd: repo,
+    git: process.execPath,
+    allowReplace: true,
+    log: procLog
+  }), er)
+  t.end()
+})