From bcd4aa1aa171efdd8774d2c36e353877b3782438 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 31 May 2020 04:48:28 +0300 Subject: [PATCH] fix: error message during offline installation PR #2596 --- .changeset/great-rocks-compare.md | 5 + .changeset/orange-news-type.md | 5 + packages/fetcher-base/src/index.ts | 1 - .../package-requester/src/packageRequester.ts | 1 - packages/tarball-fetcher/src/index.ts | 104 +++-------- packages/tarball-fetcher/test/download.ts | 174 ++---------------- 6 files changed, 46 insertions(+), 244 deletions(-) create mode 100644 .changeset/great-rocks-compare.md create mode 100644 .changeset/orange-news-type.md diff --git a/.changeset/great-rocks-compare.md b/.changeset/great-rocks-compare.md new file mode 100644 index 0000000000..6de06bad43 --- /dev/null +++ b/.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. diff --git a/.changeset/orange-news-type.md b/.changeset/orange-news-type.md new file mode 100644 index 0000000000..e84f8df4b7 --- /dev/null +++ b/.changeset/orange-news-type.md @@ -0,0 +1,5 @@ +--- +"@pnpm/fetcher-base": major +--- + +Remove `cachedTarballLocation` from `FetchOptions`. diff --git a/packages/fetcher-base/src/index.ts b/packages/fetcher-base/src/index.ts index 926c4925b0..87e68e055a 100644 --- a/packages/fetcher-base/src/index.ts +++ b/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, diff --git a/packages/package-requester/src/packageRequester.ts b/packages/package-requester/src/packageRequester.ts index 361cbe3d6a..644a56f680 100644 --- a/packages/package-requester/src/packageRequester.ts +++ b/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) => { diff --git a/packages/tarball-fetcher/src/index.ts b/packages/tarball-fetcher/src/index.ts index 3ba15bf1e5..c473ea820b 100644 --- a/packages/tarball-fetcher/src/index.ts +++ b/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, - getCredentialsByURI: mem((registry: string) => getCreds(registry)), - offline: opts.offline, - }), + download, + getCredentialsByURI: mem((registry: string) => getCreds(registry)), + offline: opts.offline, }) as FetchFunction, } } function fetchFromTarball ( ctx: { - fetchFromRemoteTarball: ( - cafs: Cafs, - dist: { - integrity?: string, - registry?: string, - tarball: string, - }, - opts: FetchOptions - ) => Promise, + download: DownloadFunction, + getCredentialsByURI: (registry: string) => { + authHeaderValue: string | undefined, + alwaysAuth: boolean | undefined, + }, + offline?: boolean, }, 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, diff --git a/packages/tarball-fetcher/test/download.ts b/packages/tarball-fetcher/test/download.ts index 69a74c76a0..1a45523f48 100644 --- a/packages/tarball-fetcher/test/download.ts +++ b/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&): 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) => { - 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', - }) - +test("don't fail when fetching a 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), - 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(), })