From 5ff6c28fab2fb6270b09ac906dd6f4e34b7dcfa3 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 8 Nov 2020 15:43:07 +0200 Subject: [PATCH] fix: retry metadata download if the received JSON is broken close #2949 PR #2971 --- .changeset/chilly-scissors-compare.md | 5 +++ packages/npm-resolver/package.json | 2 + packages/npm-resolver/src/fetch.ts | 53 +++++++++++++++++++++------ packages/npm-resolver/test/index.ts | 22 +++++++++++ pnpm-lock.yaml | 4 ++ 5 files changed, 74 insertions(+), 12 deletions(-) create mode 100644 .changeset/chilly-scissors-compare.md diff --git a/.changeset/chilly-scissors-compare.md b/.changeset/chilly-scissors-compare.md new file mode 100644 index 0000000000..78fa9ede07 --- /dev/null +++ b/.changeset/chilly-scissors-compare.md @@ -0,0 +1,5 @@ +--- +"@pnpm/npm-resolver": patch +--- + +Retry metadata download if the received JSON is broken. diff --git a/packages/npm-resolver/package.json b/packages/npm-resolver/package.json index 49451df42c..1495c7226b 100644 --- a/packages/npm-resolver/package.json +++ b/packages/npm-resolver/package.json @@ -34,11 +34,13 @@ "@pnpm/logger": "^3.1.0" }, "dependencies": { + "@pnpm/core-loggers": "workspace:^5.0.2", "@pnpm/error": "workspace:1.3.1", "@pnpm/fetching-types": "workspace:^1.0.0", "@pnpm/resolve-workspace-range": "workspace:1.0.1", "@pnpm/resolver-base": "workspace:7.0.5", "@pnpm/types": "workspace:6.3.1", + "@zkochan/retry": "^0.2.0", "encode-registry": "^3.0.0", "load-json-file": "^6.2.0", "lru-cache": "^6.0.0", diff --git a/packages/npm-resolver/src/fetch.ts b/packages/npm-resolver/src/fetch.ts index ded58cc95c..ccbfd5bca2 100644 --- a/packages/npm-resolver/src/fetch.ts +++ b/packages/npm-resolver/src/fetch.ts @@ -1,10 +1,12 @@ -import { +import { requestRetryLogger } from '@pnpm/core-loggers' +import PnpmError, { FetchError, FetchErrorRequest, FetchErrorResponse, } from '@pnpm/error' import { FetchFromRegistry, RetryTimeoutOptions } from '@pnpm/fetching-types' import { PackageMeta } from './pickPackage' +import * as retry from '@zkochan/retry' import url = require('url') interface RegistryResponse { @@ -39,21 +41,48 @@ export class RegistryResponseError extends FetchError { export default async function fromRegistry ( fetch: FetchFromRegistry, - retry: RetryTimeoutOptions, + retryOpts: RetryTimeoutOptions, pkgName: string, registry: string, authHeaderValue?: string -) { +): Promise { const uri = toUri(pkgName, registry) - const response = await fetch(uri, { authHeaderValue, retry }) as RegistryResponse - if (response.status > 400) { - const request = { - authHeaderValue, - url: uri, - } - throw new RegistryResponseError(request, response, pkgName) - } - return response.json() + const op = retry.operation(retryOpts) + return new Promise((resolve, reject) => + op.attempt(async (attempt) => { + const response = await fetch(uri, { authHeaderValue, retry: retryOpts }) as RegistryResponse + if (response.status > 400) { + const request = { + authHeaderValue, + url: uri, + } + reject(new RegistryResponseError(request, response, pkgName)) + return + } + + // Here we only retry broken JSON responses. + // Other HTTP issues are retried by the @pnpm/fetch library + try { + resolve(await response.json()) + } catch (error) { + const timeout = op.retry( + new PnpmError('BROKEN_METADATA_JSON', error.message) + ) + if (timeout === false) { + reject(op.mainError()) + return + } + requestRetryLogger.debug({ + attempt, + error, + maxRetries: retryOpts.retries!, + method: 'GET', + timeout, + url: uri, + }) + } + }) + ) } function toUri (pkgName: string, registry: string) { diff --git a/packages/npm-resolver/test/index.ts b/packages/npm-resolver/test/index.ts index 1ba3e2207c..f2ed8acf55 100644 --- a/packages/npm-resolver/test/index.ts +++ b/packages/npm-resolver/test/index.ts @@ -1427,3 +1427,25 @@ test('resolveFromNpm() should always return the name of the package that is spec expect(meta.versions).toBeTruthy() expect(meta['dist-tags']).toBeTruthy() }) + +test('request to metadata is retried if the received JSON is broken', async () => { + const registry = 'https://registry1.com/' + nock(registry) + .get('/is-positive') + .reply(200, '{') + + nock(registry) + .get('/is-positive') + .reply(200, isPositiveMeta) + + const storeDir = tempy.directory() + const resolve = createResolveFromNpm({ + retry: { retries: 1 }, + storeDir, + }) + const resolveResult = await resolve({ alias: 'is-positive', pref: '1.0.0' }, { + registry, + })! + + expect(resolveResult?.id).toBe('registry.npmjs.org/is-positive/1.0.0') +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cc9b912ebb..b50efed32b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1223,11 +1223,13 @@ importers: socks-proxy-agent: ^5.0.0 packages/npm-resolver: dependencies: + '@pnpm/core-loggers': 'link:../core-loggers' '@pnpm/error': 'link:../error' '@pnpm/fetching-types': 'link:../fetching-types' '@pnpm/resolve-workspace-range': 'link:../resolve-workspace-range' '@pnpm/resolver-base': 'link:../resolver-base' '@pnpm/types': 'link:../types' + '@zkochan/retry': 0.2.0 encode-registry: 3.0.0 load-json-file: 6.2.0 lru-cache: 6.0.0 @@ -1254,6 +1256,7 @@ importers: path-exists: 4.0.0 tempy: 1.0.0 specifiers: + '@pnpm/core-loggers': 'workspace:^5.0.2' '@pnpm/error': 'workspace:1.3.1' '@pnpm/fetch': 'workspace:^2.1.7' '@pnpm/fetching-types': 'workspace:^1.0.0' @@ -1267,6 +1270,7 @@ importers: '@types/normalize-path': ^3.0.0 '@types/semver': ^7.3.4 '@types/ssri': ^6.0.3 + '@zkochan/retry': ^0.2.0 encode-registry: ^3.0.0 load-json-file: ^6.2.0 lru-cache: ^6.0.0