Treat opendir failures as empty directories (#186)

* Tread a failing opendir as an empty dir

* Exclude specific packages

* Add a changeset

* Refactor setupTempDirectory to accept objects for dirs

* Add simple test for findNodeApiModulePathsByDependency

Author: kraenhansen

.changeset/stale-coins-drop.md

@@ -0,0 +1,5 @@
+---
+"react-native-node-api": patch
+---
+
+Treating failures when scanning filesystems for Node-API prebuilds more gracefully

packages/host/src/node/path-utils.test.ts

@@ -12,6 +12,7 @@ import {
   getLibraryName,
   isNodeApiModule,
   stripExtension,
+  findNodeApiModulePathsByDependency,
 } from "./path-utils.js";
 import { setupTempDirectory } from "./test-utils.js";
 
@@ -62,9 +63,9 @@ describe("isNodeApiModule", () => {
     assert(isNodeApiModule(path.join(tempDirectoryPath, "addon.node")));
   });
 
-  // there is no way to set ACLs on directories in Node.js on Windows with brittle powershell commands
   it(
     "returns false when directory cannot be read due to permissions",
+    // Skipping on Windows because there is no way to set ACLs on directories in Node.js on Windows without brittle powershell commands
     { skip: process.platform === "win32" },
     (context) => {
       const tempDirectoryPath = setupTempDirectory(context, {
@@ -268,24 +269,30 @@ describe("getLibraryName", () => {
 describe("findPackageDependencyPaths", () => {
   it("should find package dependency paths", (context) => {
     const tempDir = setupTempDirectory(context, {
-      "node_modules/lib-a/package.json": JSON.stringify({
-        name: "lib-a",
-        main: "index.js",
-      }),
-      "node_modules/lib-a/index.js": "",
-      "test-package/node_modules/lib-b/package.json": JSON.stringify({
-        name: "lib-b",
-        main: "index.js",
-      }),
-      "test-package/node_modules/lib-b/index.js": "",
-      "test-package/package.json": JSON.stringify({
-        name: "test-package",
-        dependencies: {
-          "lib-a": "^1.0.0",
-          "lib-b": "^1.0.0",
+      "node_modules/lib-a": {
+        "package.json": JSON.stringify({
+          name: "lib-a",
+          main: "index.js",
+        }),
+        "index.js": "",
+      },
+      "test-package": {
+        "package.json": JSON.stringify({
+          name: "test-package",
+          dependencies: {
+            "lib-a": "^1.0.0",
+            "lib-b": "^1.0.0",
+          },
+        }),
+        "src/index.js": "console.log('Hello, world!')",
+        "node_modules/lib-b": {
+          "package.json": JSON.stringify({
+            name: "lib-b",
+            main: "index.js",
+          }),
+          "index.js": "",
         },
-      }),
-      "test-package/src/index.js": "console.log('Hello, world!')",
+      },
     });
 
     const result = findPackageDependencyPaths(
@@ -367,6 +374,72 @@ describe("findNodeApiModulePaths", () => {
       path.join(tempDir, "node_modules/root.apple.node"),
     ]);
   });
+
+  it(
+    "returns empty when directory cannot be read due to permissions",
+    // Skipping on Windows because there is no way to set ACLs on directories in Node.js on Windows without brittle powershell commands
+    { skip: process.platform === "win32" },
+    async (context) => {
+      const tempDir = setupTempDirectory(context, {
+        "addon.apple.node/react-native-node-api-module": "",
+      });
+
+      removeReadPermissions(tempDir);
+      try {
+        const result = findNodeApiModulePaths({
+          fromPath: tempDir,
+          platform: "apple",
+        });
+        assert.deepEqual(await result, []);
+      } finally {
+        restoreReadPermissions(tempDir);
+      }
+    }
+  );
+});
+
+describe("findNodeApiModulePathsByDependency", () => {
+  it.only("should find Node-API paths by dependency (excluding certain packages)", async (context) => {
+    const packagesNames = ["lib-a", "lib-b", "lib-c"];
+    const tempDir = setupTempDirectory(context, {
+      "app/package.json": JSON.stringify({
+        name: "app",
+        dependencies: Object.fromEntries(
+          packagesNames.map((packageName) => [packageName, "^1.0.0"])
+        ),
+      }),
+      ...Object.fromEntries(
+        packagesNames.map((packageName) => [
+          `app/node_modules/${packageName}`,
+          {
+            "package.json": JSON.stringify({
+              name: packageName,
+              main: "index.js",
+            }),
+            "index.js": "",
+            "addon.apple.node/react-native-node-api-module": "",
+          },
+        ])
+      ),
+    });
+
+    const result = await findNodeApiModulePathsByDependency({
+      fromPath: path.join(tempDir, "app"),
+      platform: "apple",
+      includeSelf: false,
+      excludePackages: ["lib-a"],
+    });
+    assert.deepEqual(result, {
+      "lib-b": {
+        path: path.join(tempDir, "app/node_modules/lib-b"),
+        modulePaths: ["addon.apple.node"],
+      },
+      "lib-c": {
+        path: path.join(tempDir, "app/node_modules/lib-c"),
+        modulePaths: ["addon.apple.node"],
+      },
+    });
+  });
 });
 
 describe("determineModuleContext", () => {

packages/host/src/node/path-utils.ts

@@ -279,7 +279,6 @@ export const MAGIC_FILENAME = "react-native-node-api-module";
  * Default patterns to use when excluding paths from the search for Node-API modules.
  */
 export const DEFAULT_EXCLUDE_PATTERNS = [
-  /(^|\/)react-native-node-api\//,
   /(^|\/)node_modules\//,
   /(^|\/).git\//,
 ];
@@ -327,35 +326,57 @@ export async function findNodeApiModulePaths(
   const result: string[] = [];
   const pendingResults: Promise<string[]>[] = [];
 
-  for await (const dirent of await fs.promises.opendir(candidatePath)) {
+  try {
+    for await (const dirent of await fs.promises.opendir(candidatePath)) {
+      if (
+        dirent.isFile() &&
+        dirent.name === MAGIC_FILENAME &&
+        hasPlatformExtension(platform, candidatePath)
+      ) {
+        result.push(candidatePath);
+      } else if (dirent.isDirectory()) {
+        // Traverse into the child directory
+        // Pushing result into a list instead of awaiting immediately to parallelize the search
+        pendingResults.push(
+          findNodeApiModulePaths(options, path.join(suffix, dirent.name))
+        );
+      }
+    }
+  } catch (error) {
     if (
-      dirent.isFile() &&
-      dirent.name === MAGIC_FILENAME &&
-      hasPlatformExtension(platform, candidatePath)
+      error instanceof Error &&
+      "code" in error &&
+      (error.code === "ENOENT" || error.code === "EACCES")
     ) {
-      result.push(candidatePath);
-    } else if (dirent.isDirectory()) {
-      // Traverse into the child directory
-      // Pushing result into a list instead of awaiting immediately to parallelize the search
-      pendingResults.push(
-        findNodeApiModulePaths(options, path.join(suffix, dirent.name))
-      );
+      // Gracefully handling issues with reading directories
+      return [];
     }
+    throw error;
   }
   const childResults = await Promise.all(pendingResults);
   result.push(...childResults.flatMap((filePath) => filePath));
   return result;
 }
 
+/**
+ * Default package names to use when excluding packages from the search for Node-API modules.
+ */
+export const DEFAULT_EXCLUDE_PACKAGES = [
+  "react-native-node-api", // The host package itself
+  "react-native", // React Native core
+];
+
 /**
  * Finds all dependencies of the app package and their xcframeworks.
  */
 export async function findNodeApiModulePathsByDependency({
   fromPath,
   includeSelf,
+  excludePackages = DEFAULT_EXCLUDE_PACKAGES,
   ...options
 }: FindNodeApiModuleOptions & {
   includeSelf: boolean;
+  excludePackages?: string[];
 }) {
   // Find the location of each dependency
   const packagePathsByName = findPackageDependencyPaths(fromPath);
@@ -368,8 +389,9 @@ export async function findNodeApiModulePathsByDependency({
 
   // Find all their node api module paths
   const resultEntries = await Promise.all(
-    Object.entries(packagePathsByName).map(
-      async ([dependencyName, dependencyPath]) => {
+    Object.entries(packagePathsByName)
+      .filter(([name]) => !excludePackages.includes(name))
+      .map(async ([dependencyName, dependencyPath]) => {
         // Make all the xcframeworks relative to the dependency path
         const absoluteModulePaths = await findNodeApiModulePaths({
           fromPath: dependencyPath,
@@ -384,8 +406,7 @@ export async function findNodeApiModulePathsByDependency({
             ),
           },
         ] as const;
-      }
-    )
+      })
   );
   // Return an object by dependency name
   return Object.fromEntries(

packages/host/src/node/test-utils.ts

@@ -3,23 +3,32 @@ import os from "node:os";
 import fs from "node:fs";
 import path from "node:path";
 
-export function setupTempDirectory(
-  context: TestContext,
-  files: Record<string, string>
-) {
+export interface FileMap {
+  [key: string]: string | FileMap;
+}
+
+function writeFiles(fromPath: string, files: FileMap) {
+  for (const [filePath, content] of Object.entries(files)) {
+    const fullPath = path.join(fromPath, filePath);
+    fs.mkdirSync(path.dirname(fullPath), { recursive: true });
+    if (typeof content === "string") {
+      fs.writeFileSync(fullPath, content, "utf8");
+    } else {
+      writeFiles(fullPath, content);
+    }
+  }
+}
+
+export function setupTempDirectory(context: TestContext, files: FileMap) {
   const tempDirectoryPath = fs.realpathSync(
-    fs.mkdtempSync(path.join(os.tmpdir(), "babel-transform-test-"))
+    fs.mkdtempSync(path.join(os.tmpdir(), "react-native-node-api-test-"))
   );
 
   context.after(() => {
     fs.rmSync(tempDirectoryPath, { recursive: true, force: true });
   });
 
-  for (const [filePath, content] of Object.entries(files)) {
-    const fullPath = path.join(tempDirectoryPath, filePath);
-    fs.mkdirSync(path.dirname(fullPath), { recursive: true });
-    fs.writeFileSync(fullPath, content, "utf8");
-  }
+  writeFiles(tempDirectoryPath, files);
 
   return tempDirectoryPath;
 }