feat: print more info on forbid errors

close #2774
PR #2777

.changeset/fuzzy-pots-march.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/default-reporter": minor
+---
+
+Print the authorization settings (with hidden private info), when an authorization error happens during fetch.

.changeset/real-ducks-cross.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/error": minor
+---
+
+A new error class added for throwing fetch errors: FetchError.

.changeset/weak-seas-complain.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/npm-resolver": minor
+"@pnpm/tarball-fetcher": minor
+---
+
+Report whether/what authorization header was used to make the request, when the request fails with an authorization issue.

packages/default-reporter/src/index.ts

@@ -27,7 +27,7 @@ export default function (
 ) {
   if (opts.context.argv[0] === 'server') {
     const log$ = most.fromEvent<logs.Log>('data', opts.streamParser)
-    reporterForServer(log$)
+    reporterForServer(log$, opts.context.config)
     return
   }
   const outputMaxWidth = opts.reportingOptions?.outputMaxWidth ?? (process.stdout.columns && process.stdout.columns - 2) ?? 80
@@ -188,6 +188,7 @@ export function toOutput$ (
     {
       appendOnly: opts.reportingOptions?.appendOnly,
       cmd: opts.context.argv[0],
+      config: opts.context.config,
       isRecursive: opts.context.config?.['recursive'] === true,
       logLevel: opts.reportingOptions?.logLevel,
       pnpmConfig: opts.context.config,

packages/default-reporter/src/reportError.ts

@@ -1,3 +1,4 @@
+import { Config } from '@pnpm/config'
 import { Log } from '@pnpm/core-loggers'
 import PnpmError from '@pnpm/error'
 import chalk = require('chalk')
@@ -14,7 +15,7 @@ StackTracey.maxColumnWidths = {
 const highlight = chalk.yellow
 const colorPath = chalk.gray
 
-export default function reportError (logObj: Log) {
+export default function reportError (logObj: Log, config?: Config) {
   if (logObj['err']) {
     const err = logObj['err'] as (PnpmError & { stack: object })
     switch (err.code) {
@@ -42,6 +43,9 @@ export default function reportError (logObj: Log) {
         return reportLifecycleError(logObj['message'])
       case 'ERR_PNPM_UNSUPPORTED_ENGINE':
         return reportEngineError(err, logObj['message'])
+      case 'ERR_PNPM_FETCH_401':
+      case 'ERR_PNPM_FETCH_403':
+        return reportAuthError(err, logObj['message'], config)
       default:
         // Errors with unknown error codes are printed with stack trace
         if (!err.code?.startsWith?.('ERR_PNPM_')) {
@@ -282,3 +286,44 @@ To fix this issue, install the required Node version.`
   }
   return output || formatErrorSummary(err.message)
 }
+
+function reportAuthError (
+  err: Error,
+  msg: { hint?: string },
+  config?: Config
+) {
+  const foundSettings = [] as string[]
+  for (const [key, value] of Object.entries(config?.rawConfig ?? {})) {
+    if (key.startsWith('@')) {
+      foundSettings.push(`${key}=${value}`)
+      continue
+    }
+    if (
+      key.endsWith('_auth') ||
+      key.endsWith('_authToken') ||
+      key.endsWith('username') ||
+      key.endsWith('_password') ||
+      key.endsWith('always-auth')
+    ) {
+      foundSettings.push(`${key}=${hideSecureInfo(key, value)}`)
+    }
+  }
+  let output = `${formatErrorSummary(err.message)}${msg.hint ? `${EOL}${msg.hint}` : ''}
+
+`
+  if (foundSettings.length === 0) {
+    output += `No authorization settings were found in the configs.
+Try to log in to the registry by running "pnpm login"
+or add the auth tokens manually to the ~/.npmrc file.`
+  } else {
+    output += `These authorization settings were found:
+${foundSettings.join('\n')}`
+  }
+  return output
+}
+
+function hideSecureInfo (key: string, value: string) {
+  if (key.endsWith('_password')) return '[hidden]'
+  if (key.endsWith('_auth') || key.endsWith('_authToken')) return `${value.substring(0, 4)}[hidden]`
+  return value
+}

packages/default-reporter/src/reporterForClient/index.ts

@@ -41,6 +41,7 @@ export default function (
   opts: {
     appendOnly?: boolean,
     cmd: string,
+    config?: Config,
     isRecursive: boolean,
     logLevel?: LogLevel,
     pnpmConfig?: Config,
@@ -66,6 +67,7 @@ export default function (
     reportMisc(
       log$,
       {
+        config: opts.config,
         cwd,
         logLevel: opts.logLevel,
         zoomOutCurrent: opts.isRecursive,

packages/default-reporter/src/reporterForClient/reportMisc.ts

@@ -1,3 +1,4 @@
+import { Config } from '@pnpm/config'
 import { LinkLog, Log, RegistryLog } from '@pnpm/core-loggers'
 import { LogLevel } from '@pnpm/logger'
 import most = require('most')
@@ -23,6 +24,7 @@ export default (
   opts: {
     cwd: string,
     logLevel?: LogLevel,
+    config?: Config,
     zoomOutCurrent: boolean,
   }
 ) => {
@@ -36,9 +38,9 @@ export default (
           return autozoom(opts.cwd, obj.prefix, formatWarn(obj.message), opts)
         case 'error':
           if (obj['message']?.['prefix'] && obj['message']['prefix'] !== opts.cwd) {
-            return `${obj['message']['prefix']}:` + os.EOL + reportError(obj)
+            return `${obj['message']['prefix']}:` + os.EOL + reportError(obj, opts.config)
           }
-          return reportError(obj)
+          return reportError(obj, opts.config)
         default:
           return obj['message']
       }

packages/default-reporter/src/reporterForServer.ts

@@ -1,10 +1,12 @@
+import { Config } from '@pnpm/config'
 import { Log } from '@pnpm/core-loggers'
 import chalk = require('chalk')
 import most = require('most')
 import reportError from './reportError'
 
 export default function (
-  log$: most.Stream<Log>
+  log$: most.Stream<Log>,
+  config?: Config
 ) {
   log$.subscribe({
     complete: () => undefined,
@@ -19,7 +21,7 @@ export default function (
           console.log(formatWarn(log['message']))
           return
         case 'error':
-          console.log(reportError(log))
+          console.log(reportError(log, config))
           return
         case 'debug':
           return

packages/default-reporter/test/reportingErrors.ts

@@ -439,3 +439,73 @@ some hint`)
     },
   })
 })
+
+test('prints authorization error with auth settings', t => {
+  const rawConfig = {
+    '//foo.bar:_auth': '9876543219',
+    '//foo.bar:_authToken': '9876543219',
+    '//foo.bar:_password': '9876543219',
+    '//foo.bar:username': 'kiss.reka',
+    '@foo:registry': 'https://foo.bar',
+    _auth: '0123456789',
+    _authToken: '0123456789',
+    _password: '0123456789',
+    'always-auth': false,
+    username: 'nagy.gabor',
+  }
+  const output$ = toOutput$({
+    context: { argv: ['install'], config: { rawConfig } as any }, // tslint:disable-line
+    streamParser: createStreamParser(),
+  })
+
+  const err = new PnpmError('FETCH_401', 'some error', { hint: 'some hint' })
+  logger.error(err, err)
+
+  t.plan(1)
+
+  output$.take(1).map(normalizeNewline).subscribe({
+    complete: () => t.end(),
+    error: t.end,
+    next: output => {
+      t.equal(output, ERROR + ' ' + `${chalk.red('some error')}
+some hint
+
+These authorization settings were found:
+//foo.bar:_auth=9876[hidden]
+//foo.bar:_authToken=9876[hidden]
+//foo.bar:_password=[hidden]
+//foo.bar:username=kiss.reka
+@foo:registry=https://foo.bar
+_auth=0123[hidden]
+_authToken=0123[hidden]
+_password=[hidden]
+always-auth=false
+username=nagy.gabor`)
+    },
+  })
+})
+
+test('prints authorization error without auth settings, where there are none', t => {
+  const output$ = toOutput$({
+    context: { argv: ['install'], config: { rawConfig: {} } as any }, // tslint:disable-line
+    streamParser: createStreamParser(),
+  })
+
+  const err = new PnpmError('FETCH_401', 'some error', { hint: 'some hint' })
+  logger.error(err, err)
+
+  t.plan(1)
+
+  output$.take(1).map(normalizeNewline).subscribe({
+    complete: () => t.end(),
+    error: t.end,
+    next: output => {
+      t.equal(output, ERROR + ' ' + `${chalk.red('some error')}
+some hint
+
+No authorization settings were found in the configs.
+Try to log in to the registry by running "pnpm login"
+or add the auth tokens manually to the ~/.npmrc file.`)
+    },
+  })
+})

packages/error/src/index.ts

@@ -2,9 +2,44 @@ export default class PnpmError extends Error {
   public readonly code: string
   public readonly hint?: string
   public pkgsStack?: Array<{ id: string, name: string, version: string }>
-  constructor (code: string, message: string, opts?: { hint: string }) {
+  constructor (code: string, message: string, opts?: { hint?: string }) {
     super(message)
     this.code = `ERR_PNPM_${code}`
     this.hint = opts?.hint
   }
 }
+
+export type FetchErrorResponse = { status: number, statusText: string }
+
+export type FetchErrorRequest = { url: string, authHeaderValue?: string }
+
+export class FetchError extends PnpmError {
+  public readonly response: FetchErrorResponse
+  public readonly request: FetchErrorRequest
+
+  constructor (
+    request: FetchErrorRequest,
+    response: FetchErrorResponse,
+    hint?: string
+  ) {
+    let message = `GET ${request.url}: ${response.statusText} - ${response.status}`
+    const authHeaderValue = request.authHeaderValue
+      ? hideAuthInformation(request.authHeaderValue) : undefined
+    if (response.status === 401 || response.status === 403) {
+      hint = hint ? `${hint}\n\n` : ''
+      if (authHeaderValue) {
+        hint += `An authorization header was used: ${authHeaderValue}`
+      } else {
+        hint += `No authorization header was set for the request.`
+      }
+    }
+    super(`FETCH_${response.status}`, message, { hint })
+    this.request = request
+    this.response = response
+  }
+}
+
+function hideAuthInformation (authHeaderValue: string) {
+  const [authType, token] = authHeaderValue.split(' ')
+  return `${authType} ${token.substring(0, 4)}[hidden]`
+}

packages/npm-resolver/src/fetch.ts

@@ -1,4 +1,8 @@
-import PnpmError from '@pnpm/error'
+import {
+  FetchError,
+  FetchErrorRequest,
+  FetchErrorResponse,
+} from '@pnpm/error'
 import { FetchFromRegistry, RetryTimeoutOptions } from '@pnpm/fetching-types'
 import url = require('url')
 import { PackageMeta } from './pickPackage'
@@ -12,27 +16,24 @@ type RegistryResponse = {
 // https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
 const semvarRegex = new RegExp(/(.*)(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/)
 
-class RegistryResponseError extends PnpmError {
-  public readonly package: string
-  public readonly response: RegistryResponse
-  public readonly uri: string
+class RegistryResponseError extends FetchError {
+  public readonly pkgName: string
 
-  constructor (opts: {
-    package: string,
-    response: RegistryResponse,
-    uri: string,
-  }) {
-    let info = ''
-    const matched = opts.package.match(semvarRegex)
-    if (matched) {
-      info = ` Did you mean ${matched[1]}?`
+  constructor (
+    request: FetchErrorRequest,
+    response: FetchErrorResponse,
+    pkgName: string
+  ) {
+    let hint: string | undefined
+    if (response.status === 404) {
+      hint = `${pkgName} is not in the npm registry.`
+      const matched = pkgName.match(semvarRegex)
+      if (matched) {
+        hint += ` Did you mean ${matched[1]}?`
+      }
     }
-    super(
-      `REGISTRY_META_RESPONSE_${opts.response.status}`,
-      `${opts.response.status} ${opts.response.statusText}: ${opts.package} (via ${opts.uri})${info}`)
-    this.package = opts.package
-    this.response = opts.response
-    this.uri = opts.uri
+    super(request, response, hint)
+    this.pkgName = pkgName
   }
 }
 
@@ -46,11 +47,11 @@ export default async function fromRegistry (
   const uri = toUri(pkgName, registry)
   const response = await fetch(uri, { authHeaderValue, retry }) as RegistryResponse
   if (response.status > 400) {
-    throw new RegistryResponseError({
-      package: pkgName,
-      response,
-      uri,
-    })
+    const request = {
+      authToken: authHeaderValue,
+      url: uri,
+    }
+    throw new RegistryResponseError(request, response, pkgName)
   }
   return response.json()
 }

packages/npm-resolver/test/index.ts

@@ -731,10 +731,11 @@ test('error is thrown when package is not found in the registry', async t => {
     await resolveFromNpm({ alias: notExistingPackage, pref: '1.0.0' }, { registry })
     t.fail('installation should have failed')
   } catch (err) {
-    t.equal(err.message, `404 Not Found: ${notExistingPackage} (via https://registry.npmjs.org/foo)`)
-    t.equal(err['package'], notExistingPackage)
-    t.equal(err['code'], 'ERR_PNPM_REGISTRY_META_RESPONSE_404')
-    t.equal(err['uri'], `${registry}${notExistingPackage}`)
+    t.equal(err.message, `GET https://registry.npmjs.org/foo: Not Found - 404`)
+    t.equal(err['hint'], `${notExistingPackage} is not in the npm registry.`)
+    t.equal(err['pkgName'], notExistingPackage)
+    t.equal(err['code'], 'ERR_PNPM_FETCH_404')
+    t.equal(err['request']['url'], `${registry}${notExistingPackage}`)
     t.end()
   }
 })
@@ -755,10 +756,11 @@ test('extra info is shown if package has valid semver appended', async t => {
     await resolveFromNpm({ alias: notExistingPackage, pref: '1.0.0' }, { registry })
     t.fail('installation should have failed')
   } catch (err) {
-    t.equal(err.message, `404 Not Found: ${notExistingPackage} (via https://registry.npmjs.org/foo1.0.0) Did you mean foo?`)
-    t.equal(err['package'], notExistingPackage)
-    t.equal(err['code'], 'ERR_PNPM_REGISTRY_META_RESPONSE_404')
-    t.equal(err['uri'], `${registry}${notExistingPackage}`)
+    t.equal(err.message, `GET https://registry.npmjs.org/foo1.0.0: Not Found - 404`)
+    t.equal(err['hint'], `${notExistingPackage} is not in the npm registry. Did you mean foo?`)
+    t.equal(err['pkgName'], notExistingPackage)
+    t.equal(err['code'], 'ERR_PNPM_FETCH_404')
+    t.equal(err['request']['url'], `${registry}${notExistingPackage}`)
     t.end()
   }
 })
@@ -798,10 +800,11 @@ test('error is thrown when package needs authorization', async t => {
     await resolveFromNpm({ alias: 'needs-auth', pref: '*' }, { registry })
     t.fail('installation should have failed')
   } catch (err) {
-    t.equal(err.message, '403 Forbidden: needs-auth (via https://registry.npmjs.org/needs-auth)')
-    t.equal(err['package'], 'needs-auth')
-    t.equal(err['code'], 'ERR_PNPM_REGISTRY_META_RESPONSE_403')
-    t.equal(err['uri'], `${registry}needs-auth`)
+    t.equal(err.message, `GET https://registry.npmjs.org/needs-auth: Forbidden - 403`)
+    t.equal(err['hint'], `No authorization header was set for the request.`)
+    t.equal(err['pkgName'], 'needs-auth')
+    t.equal(err['code'], 'ERR_PNPM_FETCH_403')
+    t.equal(err['request']['url'], `${registry}needs-auth`)
     t.end()
   }
 })

packages/plugin-commands-installation/test/miscRecursive.ts

@@ -313,7 +313,7 @@ test('recursive installation fails when installation in one of the packages fail
   } catch (_err) {
     err = _err
   }
-  t.equal(err.code, 'ERR_PNPM_REGISTRY_META_RESPONSE_404')
+  t.equal(err.code, 'ERR_PNPM_FETCH_404')
   t.end()
 })
 

packages/server/test/index.ts

@@ -140,11 +140,11 @@ test('server errors should arrive to the client', async t => {
     )
   } catch (e) {
     caught = true
-    t.equal(e.message, '404 Not Found: not-an-existing-package (via https://registry.npmjs.org/not-an-existing-package)', 'error message delivered correctly')
-    t.equal(e.code, 'ERR_PNPM_REGISTRY_META_RESPONSE_404', 'error code delivered correctly')
-    t.ok(e.uri, 'error uri field delivered')
+    t.equal(e.message, 'GET https://registry.npmjs.org/not-an-existing-package: Not Found - 404', 'error message delivered correctly')
+    t.equal(e.hint, 'not-an-existing-package is not in the npm registry.')
+    t.equal(e.code, 'ERR_PNPM_FETCH_404', 'error code delivered correctly')
     t.ok(e.response, 'error response field delivered')
-    t.ok(e.package, 'error package field delivered')
+    t.ok(e.pkgName, 'error package field delivered')
   }
   t.ok(caught, 'exception raised correctly')
 

packages/tarball-fetcher/src/createDownloader.ts

@@ -1,5 +1,5 @@
 import { requestRetryLogger } from '@pnpm/core-loggers'
-import PnpmError from '@pnpm/error'
+import PnpmError, { FetchError } from '@pnpm/error'
 import {
   Cafs,
   DeferredManifestPromise,
@@ -13,19 +13,6 @@ import ssri = require('ssri')
 import urlLib = require('url')
 import { BadTarballError } from './errorTypes'
 
-class TarballFetchError extends PnpmError {
-  public readonly httpStatusCode: number
-  public readonly uri: string
-  public readonly response: unknown & { status: number, statusText: string }
-
-  constructor (uri: string, response: { status: number, statusText: string }) {
-    super('TARBALL_FETCH', `${response.status} ${response.statusText}: ${uri}`)
-    this.httpStatusCode = response.status
-    this.uri = uri
-    this.response = response
-  }
-}
-
 class TarballIntegrityError extends PnpmError {
   public readonly found: string
   public readonly expected: string
@@ -119,7 +106,7 @@ export default (
         try {
           resolve(await fetch(attempt))
         } catch (error) {
-          if (error.httpStatusCode === 403) {
+          if (error.response?.status === 401 || error.response?.status === 403) {
             reject(error)
           }
           const timeout = op.retry(error)
@@ -141,8 +128,9 @@ export default (
 
     async function fetch (currentAttempt: number): Promise<FetchResult> {
       try {
+        const authHeaderValue = shouldAuth ? opts.auth?.authHeaderValue : undefined
         const res = await fetchFromRegistry(url, {
-          authHeaderValue: shouldAuth ? opts.auth?.authHeaderValue : undefined,
+          authHeaderValue,
           // The fetch library can retry requests on bad HTTP responses.
           // However, it is not enough to retry on bad HTTP responses only.
           // Requests should also be retried when the tarball's integrity check fails.
@@ -152,7 +140,7 @@ export default (
         })
 
         if (res.status !== 200) {
-          throw new TarballFetchError(url, res)
+          throw new FetchError({ url, authHeaderValue }, res)
         }
 
         const contentLength = res.headers.has('content-length') && res.headers.get('content-length')

packages/tarball-fetcher/test/download.ts

@@ -326,10 +326,10 @@ test('throw error when accessing private package w/o authorization', async t =>
 
   t.ok(err)
   err = err || new Error()
-  t.equal(err.message, '403 Forbidden: http://example.com/foo.tgz')
-  t.equal(err['code'], 'ERR_PNPM_TARBALL_FETCH')
-  t.equal(err['httpStatusCode'], 403)
-  t.equal(err['uri'], 'http://example.com/foo.tgz')
+  t.equal(err.message, `GET http://example.com/foo.tgz: Forbidden - 403`)
+  t.equal(err['hint'], 'No authorization header was set for the request.')
+  t.equal(err['code'], 'ERR_PNPM_FETCH_403')
+  t.equal(err['request']['url'], 'http://example.com/foo.tgz')
 
   t.ok(scope.isDone())
   t.end()