fix: ignore broken lockfiles, unless --frozen-lockfile is set (#3217)

close #1395

.changeset/gorgeous-colts-perform.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/lockfile-file": patch
+---
+
+Throw a standard pnpm error object on broken lockfile error. The error code is `ERR_PNPM_BROKEN_LOCKFILE`.

.changeset/polite-fishes-kneel.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/get-context": major
+---
+
+`opts.autofixMergeConflicts` is replaced with `opts.frozenLockfile`.
+
+When `opts.frozenLockfile` is `false`, broken lockfiles are ignored and merge conflicts are automatically resolved.

packages/get-context/src/index.ts

@@ -67,10 +67,10 @@ interface HookOptions {
 export default async function getContext<T> (
   projects: Array<ProjectOptions & HookOptions & T>,
   opts: {
-    autofixMergeConflicts?: boolean
     force: boolean
     forceNewModules?: boolean
     forceSharedLockfile: boolean
+    frozenLockfile?: boolean
     extraBinPaths: string[]
     lockfileDir: string
     modulesDir?: string
@@ -162,9 +162,9 @@ export default async function getContext<T> (
     storeDir: opts.storeDir,
     virtualStoreDir,
     ...await readLockfileFile({
-      autofixMergeConflicts: opts.autofixMergeConflicts === true,
       force: opts.force,
       forceSharedLockfile: opts.forceSharedLockfile,
+      frozenLockfile: opts.frozenLockfile === true,
       lockfileDir: opts.lockfileDir,
       projects: importersContext.projects,
       registry: opts.registries.default,
@@ -353,7 +353,6 @@ export interface PnpmSingleContext {
 export async function getContextForSingleImporter (
   manifest: ProjectManifest,
   opts: {
-    autofixMergeConflicts?: boolean
     force: boolean
     forceNewModules?: boolean
     forceSharedLockfile: boolean
@@ -462,9 +461,9 @@ export async function getContextForSingleImporter (
     storeDir,
     virtualStoreDir,
     ...await readLockfileFile({
-      autofixMergeConflicts: opts.autofixMergeConflicts === true,
       force: opts.force,
       forceSharedLockfile: opts.forceSharedLockfile,
+      frozenLockfile: false,
       lockfileDir: opts.lockfileDir,
       projects: [{ id: importerId, rootDir: opts.dir }],
       registry: opts.registries.default,

packages/get-context/src/readLockfiles.ts

@@ -23,9 +23,9 @@ export interface PnpmContext {
 
 export default async function (
   opts: {
-    autofixMergeConflicts: boolean
     force: boolean
     forceSharedLockfile: boolean
+    frozenLockfile: boolean
     projects: Array<{
       id: string
       rootDir: string
@@ -52,13 +52,21 @@ export default async function (
   const fileReads = [] as Array<Promise<Lockfile | undefined | null>>
   let lockfileHadConflicts: boolean = false
   if (opts.useLockfile) {
-    if (opts.autofixMergeConflicts) {
+    if (!opts.frozenLockfile) {
       fileReads.push(
-        readWantedLockfileAndAutofixConflicts(opts.lockfileDir, lockfileOpts)
-          .then(({ lockfile, hadConflicts }) => {
+        (async () => {
+          try {
+            const { lockfile, hadConflicts } = await readWantedLockfileAndAutofixConflicts(opts.lockfileDir, lockfileOpts)
             lockfileHadConflicts = hadConflicts
             return lockfile
-          })
+          } catch (err) {
+            logger.warn({
+              message: `Ignoring broken lockfile at ${opts.lockfileDir}: ${err.message as string}`,
+              prefix: opts.lockfileDir,
+            })
+            return undefined
+          }
+        })()
       )
     } else {
       fileReads.push(readWantedLockfile(opts.lockfileDir, lockfileOpts))
@@ -72,7 +80,19 @@ export default async function (
     }
     fileReads.push(Promise.resolve(undefined))
   }
-  fileReads.push(readCurrentLockfile(opts.virtualStoreDir, lockfileOpts))
+  fileReads.push(
+    (async () => {
+      try {
+        return await readCurrentLockfile(opts.virtualStoreDir, lockfileOpts)
+      } catch (err) {
+        logger.warn({
+          message: `Ignoring broken lockfile at ${opts.virtualStoreDir}: ${err.message as string}`,
+          prefix: opts.lockfileDir,
+        })
+        return undefined
+      }
+    })()
+  )
   // eslint-disable-next-line @typescript-eslint/no-invalid-void-type
   const files = await Promise.all<Lockfile | null | undefined>(fileReads)
   const sopts = { lockfileVersion: LOCKFILE_VERSION }

packages/lockfile-file/src/read.ts

@@ -2,6 +2,7 @@ import {
   LOCKFILE_VERSION,
   WANTED_LOCKFILE,
 } from '@pnpm/constants'
+import PnpmError from '@pnpm/error'
 import { Lockfile } from '@pnpm/lockfile-types'
 import { DEPENDENCIES_FIELDS } from '@pnpm/types'
 import { LockfileBreakingChangeError } from './errors'
@@ -79,7 +80,7 @@ async function _read (
     hadConflicts = false
   } catch (err) {
     if (!opts.autofixMergeConflicts || !isDiff(lockfileRawContent)) {
-      throw err
+      throw new PnpmError('BROKEN_LOCKFILE', `The lockfile at "${lockfilePath}" is broken: ${err.message as string}`)
     }
     hadConflicts = true
     lockfile = autofixMergeConflicts(lockfileRawContent)

packages/supi/src/install/index.ts

@@ -136,7 +136,6 @@ export async function mutateModules (
 
   const installsOnly = projects.every((project) => project.mutation === 'install')
   opts['forceNewModules'] = installsOnly
-  opts['autofixMergeConflicts'] = !opts.frozenLockfile
   const ctx = await getContext(projects, opts)
   const rootProjectManifest = ctx.projects.find(({ id }) => id === '.')?.manifest ??
     // When running install/update on a subset of projects, the root project might not be included,

packages/supi/test/lockfile.ts

@@ -1243,6 +1243,112 @@ packages:
   expect(lockfile.dependencies['dep-of-pkg-with-1-dep']).toBe('100.1.0')
 })
 
+test('a lockfile with duplicate keys is fixed', async () => {
+  const project = prepareEmpty()
+
+  await fs.writeFile(WANTED_LOCKFILE, `\
+importers:
+  .:
+    dependencies:
+      dep-of-pkg-with-1-dep: 100.0.0
+    specifiers:
+      dep-of-pkg-with-1-dep: '100.0.0'
+lockfileVersion: ${LOCKFILE_VERSION}
+packages:
+  /dep-of-pkg-with-1-dep/100.0.0:
+    resolution: {integrity: ${getIntegrity('dep-of-pkg-with-1-dep', '100.0.0')}}
+    dev: false
+    resolution: {integrity: ${getIntegrity('dep-of-pkg-with-1-dep', '100.0.0')}}
+`, 'utf8')
+
+  const reporter = jest.fn()
+  await install({
+    dependencies: {
+      'dep-of-pkg-with-1-dep': '100.0.0',
+    },
+  }, await testDefaults({ reporter }))
+
+  const lockfile = await project.readLockfile()
+  expect(lockfile.dependencies['dep-of-pkg-with-1-dep']).toBe('100.0.0')
+
+  expect(reporter).toBeCalledWith(expect.objectContaining({
+    level: 'warn',
+    name: 'pnpm',
+    prefix: process.cwd(),
+    message: expect.stringMatching(/^Ignoring broken lockfile at .* duplicated mapping key/),
+  }))
+})
+
+test('a lockfile with duplicate keys is causes an exception, when frozenLockfile is true', async () => {
+  prepareEmpty()
+
+  await fs.writeFile(WANTED_LOCKFILE, `\
+importers:
+  .:
+    dependencies:
+      dep-of-pkg-with-1-dep: 100.0.0
+    specifiers:
+      dep-of-pkg-with-1-dep: '100.0.0'
+lockfileVersion: ${LOCKFILE_VERSION}
+packages:
+  /dep-of-pkg-with-1-dep/100.0.0:
+    resolution: {integrity: ${getIntegrity('dep-of-pkg-with-1-dep', '100.0.0')}}
+    dev: false
+    resolution: {integrity: ${getIntegrity('dep-of-pkg-with-1-dep', '100.0.0')}}
+`, 'utf8')
+
+  await expect(
+    install({
+      dependencies: {
+        'dep-of-pkg-with-1-dep': '100.0.0',
+      },
+    }, await testDefaults({ frozenLockfile: true }))
+  ).rejects.toThrow(/^The lockfile at .* is broken: duplicated mapping key/)
+})
+
+test('a broken private lockfile is ignored', async () => {
+  prepareEmpty()
+
+  const manifest = await install({
+    dependencies: {
+      'dep-of-pkg-with-1-dep': '100.0.0',
+    },
+  }, await testDefaults())
+
+  await fs.writeFile('node_modules/.pnpm/lock.yaml', `\
+importers:
+  .:
+    dependencies:
+      dep-of-pkg-with-1-dep: 100.0.0
+    specifiers:
+      dep-of-pkg-with-1-dep: '100.0.0'
+lockfileVersion: ${LOCKFILE_VERSION}
+packages:
+  /dep-of-pkg-with-1-dep/100.0.0:
+    resolution: {integrity: ${getIntegrity('dep-of-pkg-with-1-dep', '100.0.0')}}
+    dev: false
+    resolution: {integrity: ${getIntegrity('dep-of-pkg-with-1-dep', '100.0.0')}}
+`, 'utf8')
+
+  const reporter = jest.fn()
+
+  await mutateModules([
+    {
+      buildIndex: 0,
+      mutation: 'install',
+      manifest,
+      rootDir: process.cwd(),
+    },
+  ], await testDefaults({ reporter }))
+
+  expect(reporter).toBeCalledWith(expect.objectContaining({
+    level: 'warn',
+    name: 'pnpm',
+    prefix: process.cwd(),
+    message: expect.stringMatching(/^Ignoring broken lockfile at .* duplicated mapping key/),
+  }))
+})
+
 // Covers https://github.com/pnpm/pnpm/issues/2928
 test('build metadata is always ignored in versions and the lockfile is not flickering because of them', async () => {
   await addDistTag('@monorepolint/core', '0.5.0-alpha.51', 'latest')