抽丝剥茧:Electron项目fs.readdir的奇葩Bug

起因是最近在用Electron开发一个桌面端项目,有个需求是需要遍历某个文件夹下的所有JavaScript文件,对它进行AST词法语法分析,解析出元数据并写入到某个文件里。需求整体不复杂,只是细节有些麻烦,当我以为开发的差不多时,注意到一个匪夷所思的问题,查的我快怀疑人生。

缘起

什么问题呢?

原来这个需求一开始仅是遍历当前文件夹下的文件,我的获取所有JS文件的代码是这样的:

const fs = require("fs/promises");
const path = require("path");

async function getJSPathsByDir(dir) {
  const files = await fs.readdir(dir, { withFileTypes: true });
  const filePaths = files
    .filter((file) => file.isFile() && file.name.endsWith(".js"))
    .map((file) => path.join(file.path, file.name));
  console.log("filePaths:", filePaths);
  return filePaths;
}

后来需求改为要包含文件夹的子文件夹,那就需要进行递归遍历。按照我以前的做法,当然是手撸一个递归,代码并不复杂,缺点是递归可能会导致堆栈溢出:

const fs = require("fs/promises");
const path = require("path");

async function walk(dir, paths = []) {
  const files = await fs.readdir(dir, { withFileTypes: true });
  await Promise.all(
    files.map(async (file) => {
      if (file.isDirectory()) {
        await walk(path.join(dir, file.name));
      } else {
        paths.push(path.join(dir, file.name));
      }
    })
  );
}

(async () => {
  const paths = [];
  await walk("xxx", paths);
  console.log("paths:", paths.length);
})();

但做为一个紧跟时代浪潮的开发者,我知道Node.js的fs.readdir API中加了一个参数recursive,表示可以进行递归,人家代码的鲁棒性肯定比我的好多了:

const files = await fs.readdir(dir, { 
  withFileTypes: true,
  recursive: true
});

只改了一行代码,美滋滋~💯

兼容性怎么样呢?我们到Node.js的API文档里看下:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

是从v18.17.0添加的,而我本地使用的Node.js版本正是这个(好巧),我们Electron中的Node.js版本是多少呢?先看到electron的版本是27.0.4
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

Electron发布页上能找到这个版本对应的是18.17.1,比我本地的还要多一个小版本号:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

这里需要说明一下,Electron之所以优于WebView方案,是因为它内置了Chrome浏览器和Node.js,锁定了前端与后端的版本号,这样只要Chrome和Node.js本身的跨平台没有问题,理论上Electron在各个平台上都能获得统一的UI与功能体验。

而以Tauri为代表的WebView方案,则是不内置浏览器,应用程序使用宿主机的浏览器内核,开发包的体积大大减小,比如我做过的同样功能的一个项目,Electron版本有110M+,而Tauri的只有4M左右。虽然体积可以这么小,又有Rust这个性能大杀器,但在实际工作中的技术选型上,想想各种浏览器与不同版本的兼容性,换我也不敢头铁地用啊!

所以,尽管Electron有这样那样的缺点,仍是我们开发客户端的不二之选。
之所以提这个,是因为读者朋友需要明白实际项目运行的是Electron内部的Node.js,而我们本机的Node.js只负责启动Electron这个工程。

以上只是为了说明,我这里使用fs.readdir这个API新特性是没有问题的。

那么,有趣的问题来了。我启动了项目后,扫描我的某个文件夹,只打印出来前2个,而事实上,它有一百多个文件。剩下的被谁偷吃了?
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

排查

为方便排查,我将代码再度简化,提取一个单独的文件中,被Electron的Node.js端引用:

import fs from 'fs/promises'
;(async () => {
  async function getJSPathsByDir(dir) {
    const files = await fs.readdir(dir, { withFileTypes: true, recursive: true })
    console.log('Node:', process.version)
    console.log('files:', files.length)
  }
  await getJSPathsByDir('xxx')
})()

能看到控制台打印的 Node.js 版本与我们刚才查到的是一样的,文件数量为2:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug
同样的代码使用本机的Node.js执行:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

难道是这个小版本的锅?按理说不应该。但为了排除干扰,我将本机的Node.js也升级为18.17.1
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

这下就有些懵逼了!

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

追踪

目前来看,锅应该是Electron的。那么第一思路是什么?是不是人家已经解决了啊?我要不要先升个级?

没毛病。

升级Electron

将Electron的版本号换成最新版本v29.1.0
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

再看效果:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

我去,正常了!

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

不过,这个包的升级不能太草率,尤其正值发版前夕,所以还是先改代码吧,除了我上面手撸的代码外,还有个包readdirp也可以做同样的事情:

const readdirp = require("readdirp");

async function getJSPathsByDir(dir) {
  const files = await readdirp.promise(dir, { fileFilter: "*.js" });
  const filePaths = files.map((file) => file.fullPath);
  console.log("filePaths:", filePaths);
  return filePaths;
}

这两种方式,在原版本的Electron下都没有问题。

GitHub上搜索

下来接着排查。

Electron是不是有人发现了这个Bug,才进行的修复呢?

GitHub issue里瞅一瞅:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

没有,已经关闭的问题都是2022年提的问题,而我们使用的Electron的版本是2023年11月18日发布的。
那么就去代码提交记录里查下(GitHub这个功能还是很好用的):
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

符合条件的就一条,打开看下:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

修复的这个瞅着跟我们的递归没有什么关系嘛。

等等,这个文件是什么鬼?

心血来潮的收获

我们找到这个文件,目录在lib下:

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

从命名上看,这个文件是对Node.js的fs模块的一个包装。如果你对Electron有了解的话,仔细一思索,就能理解为什么会有这么个文件了。我们开发时,项目里会有许多的资源,Electron的Node.js端读取内置的文件,与正常Node.js无异,但事实上,当我们的项目打包为APP后,文件的路径与开发状态下完全不一样了。所以Electron针对打包后的文件处理,重写了fs的各个方法。

这段代码中重写readdir的部分如下:

const { readdir } = fs;
fs.readdir = function (pathArgument: string, options?: { encoding?: string | null; withFileTypes?: boolean } | null, callback?: Function) {
  const pathInfo = splitPath(pathArgument);
  if (typeof options === 'function') {
    callback = options;
    options = undefined;
  }
  if (!pathInfo.isAsar) return readdir.apply(this, arguments);
  const { asarPath, filePath } = pathInfo;

  const archive = getOrCreateArchive(asarPath);
  if (!archive) {
    const error = createError(AsarError.INVALID_ARCHIVE, { asarPath });
    nextTick(callback!, [error]);
    return;
  }

  const files = archive.readdir(filePath);
  if (!files) {
    const error = createError(AsarError.NOT_FOUND, { asarPath, filePath });
    nextTick(callback!, [error]);
    return;
  }

  if (options?.withFileTypes) {
    const dirents = [];
    for (const file of files) {
      const childPath = path.join(filePath, file);
      const stats = archive.stat(childPath);
      if (!stats) {
        const error = createError(AsarError.NOT_FOUND, { asarPath, filePath: childPath });
        nextTick(callback!, [error]);
        return;
      }
      dirents.push(new fs.Dirent(file, stats.type));
    }
    nextTick(callback!, [null, dirents]);
    return;
  }

  nextTick(callback!, [null, files]);
};

fs.promises.readdir = util.promisify(fs.readdir);

上面的判断isAsar就是判断是否打包后的归档文件,来判断是否要经Electron特殊处理。如果要处理的话,会调用Electron内部的C++代码方法进行处理。

const asar = process._linkedBinding('electron_common_asar');

const getOrCreateArchive = (archivePath: string) => {
  const isCached = cachedArchives.has(archivePath);
  if (isCached) {
    return cachedArchives.get(archivePath)!;
  }

  try {
    const newArchive = new asar.Archive(archivePath);
    cachedArchives.set(archivePath, newArchive);
    return newArchive;
  } catch {
    return null;
  }
};

我发现,这里Electron并没有对打包后的归档文件处理递归参数recursive,岂不是又一个Bug?应该提个issue提醒下。

不过,我们目前的问题,并不是它造成的,因为现在是开发状态下,上面代码可以简化为:

const fs = require("fs");
const util = require("util");

const { readdir } = fs;
fs.readdir = function () {
  return readdir.apply(this, arguments);
};

fs.promises.readdir = util.promisify(fs.readdir);

async function getJSPathsByDir(dir) {
  const files = await fs.promises.readdir(dir, {
    withFileTypes: true,
    recursive: true,
  });
  console.log("Node:", process.version);
  console.log("files:", files.length);
}

getJSPathsByDir("xxx");

对Promise了如指掌的我怎么看这代码也不会有问题,只是心血来潮执行了一下:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

我去,差点儿脑溢血!

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

好的一点是,曙光似乎就在眼前了!事实证明,心血来潮是有用的!

Node.js的源码

这时不能慌,本地切换Node.js版本为v20,同样的代码再执行下:

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

这说明Electron是被冤枉的,锅还是Node.js的!

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

Node.js你这个浓眉大眼的,居然也有Bug!呃,还偷偷修复了!

上面的情况,其实是说原生的fs.readdir有问题:

const fs = require('fs')

function getJSPathsByDir(dir) {
  fs.readdir(
    dir,
    {
      withFileTypes: true,
      recursive: true
    },
    (err, files) => {
      if (err) {
        console.error('Error:', err)
      } else {
        console.log('Node:', process.version)
        console.log('files:', files.length)
      }
    }
  )
}

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

也就是说,fs.promises.readdir并不是用util.promisify(fs.readdir)实现的!

换成同步的代码readdirSync,效果也是一样:

const fs = require('fs')

function getJSPathsByDir(dir) {
  const files = fs.readdirSync(dir, {
    withFileTypes: true,
    recursive: true
  })
  console.log('Node:', process.version)
  console.log('files:', files.length)
}

我们来到Node.js的GitHub地址,进行搜索
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

打开这两个文件,能发现,二者确实是不同的实现,不是简单地使用util.promisify进行转换。

fs.js

我们先看lib/fs.js

/**
 * Reads the contents of a directory.
 * @param {string | Buffer | URL} path
 * @param {string | {
 *   encoding?: string;
 *   withFileTypes?: boolean;
 *   recursive?: boolean;
 *   }} [options]
 * @param {(
 *   err?: Error,
 *   files?: string[] | Buffer[] | Dirent[];
 *   ) => any} callback
 * @returns {void}
 */
function readdir(path, options, callback) {
  callback = makeCallback(typeof options === 'function' ? options : callback);
  options = getOptions(options);
  path = getValidatedPath(path);
  if (options.recursive != null) {
    validateBoolean(options.recursive, 'options.recursive');
  }

  if (options.recursive) {
    callback(null, readdirSyncRecursive(path, options));
    return;
  }

  const req = new FSReqCallback();
  if (!options.withFileTypes) {
    req.oncomplete = callback;
  } else {
    req.oncomplete = (err, result) => {
      if (err) {
        callback(err);
        return;
      }
      getDirents(path, result, callback);
    };
  }
  binding.readdir(pathModule.toNamespacedPath(path), options.encoding,
                  !!options.withFileTypes, req);
}

recursive为true时,调用了一个readdirSyncRecursive函数,从这个命名上似乎可以看出有性能上的隐患。正常来说,这个函数是异步的,不应该调用同步的方法,如果文件数量过多,会把主进程占用,影响性能。

如果没有recursive,则是原来的代码,我们看到binding.readdir这个函数,凡是binding的代码,应该就是调用了C++的底层库,进行了一次『过桥』交互。

我们接着看readdirSyncRecursive,它的命名果然没有毛病,binding.readdir没有第4个参数,是完全同步的,这个风险是显而易见的:

/**
 * An iterative algorithm for reading the entire contents of the `basePath` directory.
 * This function does not validate `basePath` as a directory. It is passed directly to
 * `binding.readdir`.
 * @param {string} basePath
 * @param {{ encoding: string, withFileTypes: boolean }} options
 * @returns {string[] | Dirent[]}
 */
function readdirSyncRecursive(basePath, options) {
  const withFileTypes = Boolean(options.withFileTypes);
  const encoding = options.encoding;

  const readdirResults = [];
  const pathsQueue = [basePath];

  function read(path) {
    const readdirResult = binding.readdir(
      pathModule.toNamespacedPath(path),
      encoding,
      withFileTypes,
    );

    if (readdirResult === undefined) {
      return;
    }

    if (withFileTypes) {
      // Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
      // The first array is the names, and the second array is the types.
      // They are guaranteed to be the same length; hence, setting `length` to the length
      // of the first array within the result.
      const length = readdirResult[0].length;
      for (let i = 0; i < length; i++) {
        const dirent = getDirent(path, readdirResult[0][i], readdirResult[1][i]);
        ArrayPrototypePush(readdirResults, dirent);
        if (dirent.isDirectory()) {
          ArrayPrototypePush(pathsQueue, pathModule.join(dirent.parentPath, dirent.name));
        }
      }
    } else {
      for (let i = 0; i < readdirResult.length; i++) {
        const resultPath = pathModule.join(path, readdirResult[i]);
        const relativeResultPath = pathModule.relative(basePath, resultPath);
        const stat = binding.internalModuleStat(resultPath);
        ArrayPrototypePush(readdirResults, relativeResultPath);
        // 1 indicates directory
        if (stat === 1) {
          ArrayPrototypePush(pathsQueue, resultPath);
        }
      }
    }
  }

  for (let i = 0; i < pathsQueue.length; i++) {
    read(pathsQueue[i]);
  }

  return readdirResults;
}

fs/promises.js

lib/internal/fs/promises.js中,我们看到binding.readdir的第4个参数是kUsePromises,表明是个异步的处理。

const { kUsePromises } = binding;

async function readdir(path, options) {
  options = getOptions(options);
  path = getValidatedPath(path);
  if (options.recursive) {
    return readdirRecursive(path, options);
  }
  const result = await PromisePrototypeThen(
    binding.readdir(
      pathModule.toNamespacedPath(path),
      options.encoding,
      !!options.withFileTypes,
      kUsePromises,
    ),
    undefined,
    handleErrorFromBinding,
  );
  return options.withFileTypes ?
    getDirectoryEntriesPromise(path, result) :
    result;
}

当传递了recursive参数时,将调用readdirRecursive,而readdirRecursive的代码与readdirSyncRecursive的大同小异,有兴趣的可以读一读:

async function readdirRecursive(originalPath, options) {
  const result = [];
  const queue = [
    [
      originalPath,
      await PromisePrototypeThen(
        binding.readdir(
          pathModule.toNamespacedPath(originalPath),
          options.encoding,
          !!options.withFileTypes,
          kUsePromises,
        ),
        undefined,
        handleErrorFromBinding,
      ),
    ],
  ];


  if (options.withFileTypes) {
    while (queue.length > 0) {
      // If we want to implement BFS make this a `shift` call instead of `pop`
      const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
      for (const dirent of getDirents(path, readdir)) {
        ArrayPrototypePush(result, dirent);
        if (dirent.isDirectory()) {
          const direntPath = pathModule.join(path, dirent.name);
          ArrayPrototypePush(queue, [
            direntPath,
            await PromisePrototypeThen(
              binding.readdir(
                direntPath,
                options.encoding,
                true,
                kUsePromises,
              ),
              undefined,
              handleErrorFromBinding,
            ),
          ]);
        }
      }
    }
  } else {
    while (queue.length > 0) {
      const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
      for (const ent of readdir) {
        const direntPath = pathModule.join(path, ent);
        const stat = binding.internalModuleStat(direntPath);
        ArrayPrototypePush(
          result,
          pathModule.relative(originalPath, direntPath),
        );
        if (stat === 1) {
          ArrayPrototypePush(queue, [
            direntPath,
            await PromisePrototypeThen(
              binding.readdir(
                pathModule.toNamespacedPath(direntPath),
                options.encoding,
                false,
                kUsePromises,
              ),
              undefined,
              handleErrorFromBinding,
            ),
          ]);
        }
      }
    }
  }

  return result;
}

fs.js的提交记录

我们搜索readdir的提交记录,能发现这两篇都与深度遍历有关:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

其中下面的这个,正是我们这次问题的罪魁祸首。

刚才看到的fs.js中的readdirSyncRecursive里这段长长的注释,正是这次提交里添加的:
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

从代码对比上,我们就能看出为什么我们的代码遍历的程序为2了,因为readdirResult是个二维数组,它的长度就是2啊。取它的第一个元素的长度才是正解。坑爹!

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

也就是说,如果不使用withFileTypes这个参数,得到的结果是没有问题的:

const fs = require('fs')

function getJSPathsByDir(dir) {
  fs.readdir(
    dir,
    {
      // withFileTypes: true,
      recursive: true
    },
    (err, files) => {
      if (err) {
        console.error('Error:', err)
      } else {
        console.log('Node:', process.version)
        console.log('files:', files.length)
      }
    }
  )
}

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

发版记录

我们在Node.js的发版记录中,找到这条提交记录,也就是说,v18.19.0才修复这个问题。

抽丝剥茧:Electron项目fs.readdir的奇葩Bug
抽丝剥茧:Electron项目fs.readdir的奇葩Bug

而Electron只有Node.js更新到v20后,这个功能才修复。

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

而从Electron与Node.js的版本对应上来看,得更新到v29了。

只是需要注意的是,像前文提过的,如果是遍历的是当前项目的内置文件,Electron并没有支持这个新的参数,在打包后会出现Bug。

fs的阻塞同步

其实有人提过一个issue

抽丝剥茧:Electron项目fs.readdir的奇葩Bug

确实是个风险点。所以,建议Node.js开发者老老实实使用fs/promises代替fs,而Electron用户由于坑爹的fs包裹,还是不要用这个特性了。

总结

本次问题的起因是我的一个Electron项目,使用了一个Node.js fs API的一个新参数recursive递归遍历文件夹,偶然间发现返回的文件数量不对,就开始排查问题。

首先,我选择了升级Electron的包版本,发现从v27.0.4升级到最新版本v29.1.0后,问题解决了。但由于发版在即,不能冒然升级这么大件的东西,所以先使用readdirp这个第三方包处理。

接着排查问题出现的原因。我到Electron的GitHub上搜索issue,只找到一条近期的提交,但看提交信息,不像是我们的目标。我注意到这条提交的修改文件(asar-fs-wrapper.ts),是Electron针对Node.js的fs API的包装,意识到这是Electron为了解决打包前后内置文件路径的问题,心血来潮之下,将其中核心代码简化后,测试发现问题出在fs.promises.readdir的重写上,继而锁定了Node.js版本v18.17.1fs.readdir有Bug。

下一步,继续看Node.js的源码,确定了fs.promisesfs是两套不同的实现,不是简单地使用util.promisify进行转换。并在fs的代码找到关于recursive递归的核心代码readdirSyncRecursive。又在提交记录里,找到这段代码的修复记录。仔细阅读代码对比后,找到了返回数量为2的真正原因。

我们在Node.js的发版记录中,找到了这条记录的信息,确定了v18.19.0才修复这个问题。而内嵌Node.js的Electron,只有v29版本起才修复。

不过需要注意的是,如果是遍历的是当前项目的内置文件,由于Electron并没有支持这个新的参数,在打包后会出现Bug。而且由于fs.readdir使用recursive时是同步的,Electron重写的fs.promises.readdir最终调用的是它,可能会有隐性的性能风险。

本次定位问题走了些弯路,一开始将目标锁定到Electron上,主要是它重写fs的锅,如果我在代码中用的fs.readdirSync,那么可能会更早在Node.js上查起。谁能想到我调用的fs.promises.readdir不是真正的fs.promises.readdir呢?

最后,针对此次问题,我们有以下启示:

  1. Node.js项目,尽可能升级Node.js版本,至少要到LTS,比如今年已经主打v20了,仍继续使用v18已经落伍了。
  2. 平时注意常用工具链的发版记录,尤其是修复的Bug,看是否有与你相关的内容。
  3. Node.js能用异步就用异步,同步的会有性能隐患。
  4. Node.js的异步API,如果有官方重写的promises方法,就不要使用util.promisify API进行转换。
  5. 不要随便重写底层API,即使是Electron这样牛逼的框架也免不了会踩坑。

PS:我给Electron提了个issue,一是让他们给fs.readdir添加recursive参数的实现,二是让他们注意下重写时fs.promises.readdir的性能风险。

原文链接:https://juejin.cn/post/7343804530994593804 作者:纪轻昀

(0)
上一篇 2024年3月8日 下午4:54
下一篇 2024年3月8日 下午5:07

相关推荐

发表回复

登录后才能评论