fix: error message during offline installation

PR #2596

.changeset/great-rocks-compare.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/tarball-fetcher": major
+---
+
+Remove `cachedTarballLocation` from `FetchOptions`. pnpm v5 doesn't store the package tarball files in the cache anymore.

.changeset/orange-news-type.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/fetcher-base": major
+---
+
+Remove `cachedTarballLocation` from `FetchOptions`.

packages/fetcher-base/src/index.ts

@@ -8,7 +8,6 @@ export type Cafs = {
 }
 
 export interface FetchOptions {
-  cachedTarballLocation: string,
   manifest?: DeferredManifestPromise,
   lockfileDir: string,
   onStart?: (totalSize: number | null, attempt: number) => void,

packages/package-requester/src/packageRequester.ts

@@ -434,7 +434,6 @@ function fetchToStore (
         opts.pkgId,
         opts.resolution,
         {
-          cachedTarballLocation: path.join(ctx.storeDir, opts.pkgId, 'packed.tgz'),
           lockfileDir: opts.lockfileDir,
           manifest: fetchManifest,
           onProgress: (downloaded) => {

packages/tarball-fetcher/src/index.ts

@@ -6,7 +6,6 @@ import {
   FetchOptions,
   FetchResult,
 } from '@pnpm/fetcher-base'
-import { globalWarn } from '@pnpm/logger'
 import getCredentialsByURI = require('credentials-by-uri')
 import mem = require('mem')
 import fs = require('mz/fs')
@@ -58,26 +57,21 @@ export default function (
   const getCreds = getCredentialsByURI.bind(null, opts.rawConfig)
   return {
     tarball: fetchFromTarball.bind(null, {
-      fetchFromRemoteTarball: fetchFromRemoteTarball.bind(null, {
+      download,
-        download,
+      getCredentialsByURI: mem((registry: string) => getCreds(registry)),
-        getCredentialsByURI: mem((registry: string) => getCreds(registry)),
+      offline: opts.offline,
-        offline: opts.offline,
-      }),
     }) as FetchFunction,
   }
 }
 
 function fetchFromTarball (
   ctx: {
-    fetchFromRemoteTarball: (
+    download: DownloadFunction,
-      cafs: Cafs,
+    getCredentialsByURI: (registry: string) => {
-      dist: {
+      authHeaderValue: string | undefined,
-        integrity?: string,
+      alwaysAuth: boolean | undefined,
-        registry?: string,
+    },
-        tarball: string,
+    offline?: boolean,
-      },
-      opts: FetchOptions
-    ) => Promise<FetchResult>,
   },
   cafs: Cafs,
   resolution: {
@@ -94,7 +88,20 @@ function fetchFromTarball (
       manifest: opts.manifest,
     })
   }
-  return ctx.fetchFromRemoteTarball(cafs, resolution, opts)
+  if (ctx.offline) {
+    throw new PnpmError('NO_OFFLINE_TARBALL',
+      `A package is missing from the store but cannot download it in offline mode. The missing package may be downloaded from ${resolution.tarball}.`)
+  }
+  const auth = resolution.registry ? ctx.getCredentialsByURI(resolution.registry) : undefined
+  return ctx.download(resolution.tarball, {
+    auth,
+    cafs,
+    integrity: resolution.integrity,
+    manifest: opts.manifest,
+    onProgress: opts.onProgress,
+    onStart: opts.onStart,
+    registry: resolution.registry,
+  })
 }
 
 const isAbsolutePath = /^[/]|^[A-Za-z]:/
@@ -104,71 +111,6 @@ function resolvePath (where: string, spec: string) {
   return path.resolve(where, spec)
 }
 
-async function fetchFromRemoteTarball (
-  ctx: {
-    offline?: boolean,
-    download: DownloadFunction,
-    getCredentialsByURI: (registry: string) => {
-      authHeaderValue: string | undefined,
-      alwaysAuth: boolean | undefined,
-    },
-  },
-  cafs: Cafs,
-  dist: {
-    integrity?: string,
-    registry?: string,
-    tarball: string,
-  },
-  opts: FetchOptions
-) {
-  try {
-    return await fetchFromLocalTarball(cafs, opts.cachedTarballLocation, {
-      integrity: dist.integrity,
-      manifest: opts.manifest,
-    })
-  } catch (err) {
-    // ignore errors for missing files or broken/partial archives
-    switch (err.code) {
-      case 'Z_BUF_ERROR':
-        if (ctx.offline) {
-          throw new PnpmError(
-            'CORRUPTED_TARBALL',
-            `The cached tarball at "${opts.cachedTarballLocation}" is corrupted. Cannot redownload it as offline mode was requested.`
-          )
-        }
-        globalWarn(`Redownloading corrupted cached tarball: ${opts.cachedTarballLocation}`)
-        break
-      case 'EINTEGRITY':
-        if (ctx.offline) {
-          throw new PnpmError(
-            'BAD_TARBALL_CHECKSUM',
-            `The cached tarball at "${opts.cachedTarballLocation}" did not pass the integrity check. Cannot redownload it as offline mode was requested.`
-          )
-        }
-        globalWarn(`The cached tarball at "${opts.cachedTarballLocation}" did not pass the integrity check. Redownloading.`)
-        break
-      case 'ENOENT':
-        if (ctx.offline) {
-          throw new PnpmError('NO_OFFLINE_TARBALL', `Could not find ${opts.cachedTarballLocation} in local registry mirror`)
-        }
-        break
-      default:
-        throw err
-    }
-
-    const auth = dist.registry ? ctx.getCredentialsByURI(dist.registry) : undefined
-    return ctx.download(dist.tarball, {
-      auth,
-      cafs,
-      integrity: dist.integrity,
-      manifest: opts.manifest,
-      onProgress: opts.onProgress,
-      onStart: opts.onStart,
-      registry: dist.registry,
-    })
-  }
-}
-
 async function fetchFromLocalTarball (
   cafs: Cafs,
   tarball: string,

packages/tarball-fetcher/test/download.ts

@@ -42,7 +42,6 @@ test('fail when tarball size does not match content-length', async t => {
   process.chdir(tempy.directory())
   t.comment(`temp dir ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   const resolution = {
     // Even though the integrity of the downloaded tarball
     // will not match this value, the error will be about
@@ -54,7 +53,6 @@ test('fail when tarball size does not match content-length', async t => {
 
   try {
     await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
       lockfileDir: process.cwd(),
     })
     t.fail('should have failed')
@@ -70,88 +68,6 @@ test('fail when tarball size does not match content-length', async t => {
   }
 })
 
-test('redownload the tarball when the one in cache does not satisfy integrity', async t => {
-  const scope = nock(registry)
-    .get('/babel-helper-hoist-variables-6.24.1.tgz')
-    .times(1)
-    .replyWithFile(200, tarballPath, {
-      'Content-Length': tarballSize.toString(),
-    })
-
-  const cacheDir = tempy.directory()
-  t.comment(`temp dir ${cacheDir}`)
-
-  const cachedTarballLocation = path.join(cacheDir, 'cache.tgz')
-  await cpFile(
-    path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz'),
-    cachedTarballLocation
-  )
-
-  const resolution = {
-    integrity: tarballIntegrity,
-    tarball: `${registry}babel-helper-hoist-variables-6.24.1.tgz`,
-  }
-
-  t.plan(3)
-  function reporter (log: LogBase & {level: string, name: string, message: string}) {
-    if (log.level === 'warn' && log.name === 'pnpm:global' && log.message.startsWith(`The cached tarball at "${cachedTarballLocation}"`)) {
-      t.pass('warning logged')
-    }
-  }
-  streamParser.on('data', reporter as any) // tslint:disable-line:no-any
-  const { filesIndex } = await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
-    lockfileDir: process.cwd(),
-  })
-  streamParser.removeListener('data', reporter as any) // tslint:disable-line:no-any
-
-  const pkgJsonIntegrity = await filesIndex['package.json'].generatingIntegrity
-  t.equal((await readPackage(getFilePathByModeInCafs(pkgJsonIntegrity, filesIndex['package.json'].mode))).version, '6.24.1')
-
-  t.ok(scope.isDone())
-  t.end()
-})
-
-test('fail when the tarball in the cache does not pass integrity check in offline mode', async t => {
-  const cacheDir = tempy.directory()
-  t.comment(`temp dir ${cacheDir}`)
-
-  const cachedTarballLocation = path.join(cacheDir, 'cache.tgz')
-  await cpFile(
-    path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz'),
-    cachedTarballLocation
-  )
-
-  const resolution = {
-    integrity: tarballIntegrity,
-    tarball: `${registry}babel-helper-hoist-variables-6.24.1.tgz`,
-  }
-
-  let err!: Error
-  try {
-    const fetch = createFetcher({
-      fetchRetries: 1,
-      fetchRetryMaxtimeout: 100,
-      fetchRetryMintimeout: 0,
-      offline: true,
-      rawConfig: {
-        registry,
-      },
-      registry,
-    })
-    await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
-      lockfileDir: process.cwd(),
-    })
-  } catch (_err) {
-    err = _err
-  }
-
-  t.ok(err)
-  t.equal(err['code'], 'ERR_PNPM_BAD_TARBALL_CHECKSUM')
-  t.end()
-})
-
 test('retry when tarball size does not match content-length', async t => {
   const scope = nock(registry)
     .get('/foo.tgz')
@@ -168,11 +84,9 @@ test('retry when tarball size does not match content-length', async t => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached.tgz')
   const resolution = { tarball: 'http://example.com/foo.tgz' }
 
   const result = await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
     lockfileDir: process.cwd(),
   })
 
@@ -181,53 +95,6 @@ test('retry when tarball size does not match content-length', async t => {
   t.end()
 })
 
-test('redownload incomplete cached tarballs', async t => {
-  if (+process.version.split('.')[0].substr(1) >= 10) {
-    // TODO: investigate why the following error happens on Node>=10:
-    // node[30990]: ../src/node_file.cc:1715:void node::fs::WriteBuffer(const v8::FunctionCallbackInfo<v8::Value>&): Assertion `args[3]->IsInt32()' failed.
-    t.skip('This test is skipped on Node.js 10')
-    t.end()
-    return
-  }
-  const scope = nock(registry)
-    .get('/foo.tgz')
-    .replyWithFile(200, tarballPath, {
-      'Content-Length': tarballSize.toString(),
-    })
-
-  process.chdir(tempy.directory())
-  t.comment(`testing in ${process.cwd()}`)
-
-  const cachedTarballLocation = path.resolve('cached')
-  const cachedTarballFd = await fs.open(cachedTarballLocation, 'w')
-  const tarballData = await fs.readFile(tarballPath)
-  await fs.write(cachedTarballFd, tarballData, 0, tarballSize / 2)
-  await fs.close(cachedTarballFd)
-
-  const resolution = { tarball: 'http://example.com/foo.tgz' }
-
-  t.plan(2)
-  function reporter (log: LogBase & {level: string, name: string, message: string}) {
-    if (log.level === 'warn' && log.name === 'pnpm:store' && log.message.startsWith(`Redownloading corrupted cached tarball: ${cachedTarballLocation}`)) {
-      t.pass('warning logged')
-    }
-  }
-  streamParser.on('data', reporter as any) // tslint:disable-line:no-any
-  try {
-    await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
-      lockfileDir: process.cwd(),
-    })
-  } catch (err) {
-    nock.cleanAll()
-    t.fail(err)
-  }
-  streamParser.removeListener('data', reporter as any) // tslint:disable-line:no-any
-
-  t.ok(scope.isDone())
-  t.end()
-})
-
 test('fail when integrity check fails two times in a row', async t => {
   const scope = nock(registry)
     .get('/foo.tgz')
@@ -239,7 +106,6 @@ test('fail when integrity check fails two times in a row', async t => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   const resolution = {
     integrity: tarballIntegrity,
     tarball: 'http://example.com/foo.tgz',
@@ -247,7 +113,6 @@ test('fail when integrity check fails two times in a row', async t => {
 
   try {
     await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
       lockfileDir: process.cwd(),
     })
     t.fail('should have failed')
@@ -277,7 +142,6 @@ test('retry when integrity check fails', async t => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   const resolution = {
     integrity: tarballIntegrity,
     tarball: 'http://example.com/foo.tgz',
@@ -285,7 +149,6 @@ test('retry when integrity check fails', async t => {
 
   const params: Array<[number | null, number]> = []
   await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
     lockfileDir: process.cwd(),
     onStart (size, attempts) {
       params.push([size, attempts])
@@ -303,7 +166,6 @@ test('fail when integrity check of local file fails', async (t) => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   await cpFile(
     path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz'),
     path.resolve('tar.tgz')
@@ -316,7 +178,6 @@ test('fail when integrity check of local file fails', async (t) => {
   let err: Error | null = null
   try {
     await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
       lockfileDir: process.cwd(),
     })
   } catch (_err) {
@@ -337,7 +198,6 @@ test("don't fail when integrity check of local file succeeds", async (t) => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   const localTarballLocation = path.resolve('tar.tgz')
   await cpFile(
     path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz'),
@@ -349,7 +209,6 @@ test("don't fail when integrity check of local file succeeds", async (t) => {
   }
 
   const { filesIndex } = await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
     lockfileDir: process.cwd(),
   })
 
@@ -358,26 +217,27 @@ test("don't fail when integrity check of local file succeeds", async (t) => {
   t.end()
 })
 
-test("don't fail when the cache tarball does not exist", async (t) => {
+test("don't fail when fetching a local tarball in offline mode", async (t) => {
-  nock(registry)
-    .get('/foo.tgz')
-    .times(1)
-    .replyWithFile(200, path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz'), {
-      'Content-Length': '1194',
-    })
-
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('dir', 'cached')
   const tarballAbsoluteLocation = path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz')
   const resolution = {
     integrity: await getFileIntegrity(tarballAbsoluteLocation),
-    tarball: `${registry}foo.tgz`,
+    tarball: `file:${tarballAbsoluteLocation}`,
   }
 
+  const fetch = createFetcher({
+    fetchRetries: 1,
+    fetchRetryMaxtimeout: 100,
+    fetchRetryMintimeout: 0,
+    offline: true,
+    rawConfig: {
+      registry,
+    },
+    registry,
+  })
   const { filesIndex } = await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
     lockfileDir: process.cwd(),
   })
 
@@ -386,11 +246,10 @@ test("don't fail when the cache tarball does not exist", async (t) => {
   t.end()
 })
 
-test('fail when the cache tarball does not exist in offline mode', async (t) => {
+test('fail when trying to fetch a non-local tarball in offline mode', async (t) => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('dir', 'cached')
   const tarballAbsoluteLocation = path.join(__dirname, 'tars', 'babel-helper-hoist-variables-7.0.0-alpha.10.tgz')
   const resolution = {
     integrity: await getFileIntegrity(tarballAbsoluteLocation),
@@ -410,7 +269,6 @@ test('fail when the cache tarball does not exist in offline mode', async (t) =>
       registry,
     })
     await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
       lockfileDir: process.cwd(),
     })
   } catch (_err) {
@@ -435,14 +293,12 @@ test('retry on server error', async t => {
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   const resolution = {
     integrity: tarballIntegrity,
     tarball: 'http://example.com/foo.tgz',
   }
 
   const index = await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
     lockfileDir: process.cwd(),
   })
 
@@ -460,7 +316,6 @@ test('throw error when accessing private package w/o authorization', async t =>
   process.chdir(tempy.directory())
   t.comment(`testing in ${process.cwd()}`)
 
-  const cachedTarballLocation = path.resolve('cached')
   const resolution = {
     integrity: tarballIntegrity,
     tarball: 'http://example.com/foo.tgz',
@@ -470,7 +325,6 @@ test('throw error when accessing private package w/o authorization', async t =>
 
   try {
     await fetch.tarball(cafs, resolution, {
-      cachedTarballLocation,
       lockfileDir: process.cwd(),
     })
   } catch (_err) {
@@ -517,7 +371,6 @@ test('accessing private packages', async t => {
     registry,
   })
 
-  const cachedTarballLocation = path.resolve('cached')
   const resolution = {
     integrity: tarballIntegrity,
     registry,
@@ -525,7 +378,6 @@ test('accessing private packages', async t => {
   }
 
   const index = await fetch.tarball(cafs, resolution, {
-    cachedTarballLocation,
     lockfileDir: process.cwd(),
   })