fix(store): don't prematurely bail out of adding source files if ENOENT is thrown (#6932)

Broken symbolic links will cause a `stat'-call to throw resulting in an
arbitrary amount of promises that won't get to settle before the index is
returned.

Processing subdirectories in the following iteration in the event loop makes
this consistently reproducible.

---------

Co-authored-by: Martin Madsen <mj@blackbird.online>
Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/breezy-carpets-obey.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/store.cafs": patch
+---
+
+Don't prematurely bail out of adding source files if ENOENT is thrown [#6932](https://github.com/pnpm/pnpm/pull/6932).

store/cafs/src/addFilesFromDir.ts

@@ -1,4 +1,4 @@
-import { promises as fs } from 'fs'
+import { promises as fs, type Stats } from 'fs'
 import path from 'path'
 import type {
   DeferredManifestPromise,
@@ -39,39 +39,41 @@ async function _retrieveFileIntegrities (
   index: FilesIndex,
   deferredManifest?: DeferredManifestPromise
 ) {
-  try {
-    const files = await fs.readdir(currDir)
-    await Promise.all(files.map(async (file) => {
-      const fullPath = path.join(currDir, file)
-      const stat = await fs.stat(fullPath)
-      if (stat.isDirectory()) {
-        await _retrieveFileIntegrities(cafs, rootDir, fullPath, index)
+  const files = await fs.readdir(currDir, { withFileTypes: true })
+  await Promise.all(files.map(async (file) => {
+    const fullPath = path.join(currDir, file.name)
+    if (file.isDirectory()) {
+      await _retrieveFileIntegrities(cafs, rootDir, fullPath, index)
+      return
+    }
+    if (file.isFile()) {
+      const relativePath = path.relative(rootDir, fullPath)
+      let stat: Stats
+      try {
+        stat = await fs.stat(fullPath)
+      } catch (err: any) { // eslint-disable-line
+        if (err.code !== 'ENOENT') {
+          throw err
+        }
         return
       }
-      if (stat.isFile()) {
-        const relativePath = path.relative(rootDir, fullPath)
-        const writeResult = limit(async () => {
-          if ((deferredManifest != null) && rootDir === currDir && file === 'package.json') {
-            const buffer = await gfs.readFile(fullPath)
-            parseJsonBuffer(buffer, deferredManifest)
-            return cafs.addBuffer(buffer, stat.mode)
-          }
-          if (stat.size < MAX_BULK_SIZE) {
-            const buffer = await gfs.readFile(fullPath)
-            return cafs.addBuffer(buffer, stat.mode)
-          }
-          return cafs.addStream(gfs.createReadStream(fullPath), stat.mode)
-        })
-        index[relativePath] = {
-          mode: stat.mode,
-          size: stat.size,
-          writeResult,
+      const writeResult = limit(async () => {
+        if ((deferredManifest != null) && rootDir === currDir && file.name === 'package.json') {
+          const buffer = await gfs.readFile(fullPath)
+          parseJsonBuffer(buffer, deferredManifest)
+          return cafs.addBuffer(buffer, stat.mode)
         }
+        if (stat.size < MAX_BULK_SIZE) {
+          const buffer = await gfs.readFile(fullPath)
+          return cafs.addBuffer(buffer, stat.mode)
+        }
+        return cafs.addStream(gfs.createReadStream(fullPath), stat.mode)
+      })
+      index[relativePath] = {
+        mode: stat.mode,
+        size: stat.size,
+        writeResult,
       }
-    }))
-  } catch (err: any) { // eslint-disable-line
-    if (err.code !== 'ENOENT') {
-      throw err
     }
-  }
+  }))
 }

store/cafs/test/fixtures/broken-symlink/doesnt-exist

@@ -0,0 +1 @@
+../doesnt-exist

store/cafs/test/index.ts

@@ -43,6 +43,16 @@ describe('cafs', () => {
     expect(fs.readFileSync(filePath, 'utf8')).toBe('foo\n')
     expect(await manifest.promise).toEqual(undefined)
   })
+
+  it('ignores broken symlinks when traversing subdirectories', async () => {
+    const storeDir = tempy.directory()
+    const srcDir = path.join(__dirname, 'fixtures/broken-symlink')
+    const manifest = pDefer<DependencyManifest>()
+    const addFiles = async () => createCafs(storeDir).addFilesFromDir(srcDir, manifest)
+
+    const filesIndex = await addFiles()
+    expect(filesIndex['subdir/should-exist.txt']).toBeDefined()
+  })
 })
 
 describe('checkPkgFilesIntegrity()', () => {