fix: retry metadata download if the received JSON is broken

close #2949
PR #2971

.changeset/chilly-scissors-compare.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/npm-resolver": patch
+---
+
+Retry metadata download if the received JSON is broken.

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",

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<PackageMeta> {
   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) {

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')
+})

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