berry: [Bug] fs functions that use buffer encoding fail

  • I’d be willing to implement a fix

Describe the bug

Yarn fs reimplementation assumes fs functions only accept strings as input, instead they could be Buffer or URL for example

The error you get is

TypeError: p.match is not a function
    at Function.resolveVirtual (/Users/morse/Documents/GitHub/sterblue1/.pnp.js:125678:21)
    at VirtualFS.mapToBase (/Users/morse/Documents/GitHub/sterblue1/.pnp.js:125715:22)
    at VirtualFS.readdirPromise (/Users/morse/Documents/GitHub/sterblue1/.pnp.js:125588:44)
    at PosixFS.readdirPromise (/Users/morse/Documents/GitHub/sterblue1/.pnp.js:125588:24)

To Reproduce

const fs = require("fs");

require("./.pnp.js").setup();

const bufdir = Buffer.from(".");
fs.readdir(bufdir, { encoding: "buffer" }, x => console.log(x));
Reproduction 
// Sherlock reproduction

Screenshots

If applicable, add screenshots to help explain your problem.

Environment if relevant (please complete the following information):

  • OS: [e.g. OSX, Linux, Windows, …]
  • Node version [e.g. 8.15.0, 10.15.1, …]
  • Yarn version [e.g. 2.0.0-rc1, …]

Additional context

#899 is a similar problem

node-dir npm package (2 million monthly downloads) files function fails because of this

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 7
  • Comments: 15 (2 by maintainers)

Commits related to this issue

Most upvoted comments

rant My minimum reproducible repo would include `serverless` and `serverless-esbuild` which is sub-optimal, it can be easily dodged by maintainers so I am not providing it here. And I don't bother learning yarn internals enough to pin point the exact place where it does codegen for `.pnp.cjs`. Nobody else can do this as efficiently as current contributors.

Here is a patch instead.

moar rant

Couldn’t resist and finally took a look. Now that I’m in the rabbit hole, it’s @yarnpkg/fslib who’s responsible of the affected code segment.

I just don’t know how tf an unexpected buffer input and an expected string input is simply expressed as PortablePath in the parameter.

Come on, guys.

Workaround

Apply the following patch in your .pnp.cjs,

@@ -32543,10 +32543,11 @@ class VirtualFS extends ProxiedFS {
   mapToBase(p) {
+    const pathString = `${p}`;
     if (pathString === ``)
       return p;
-    if (this.pathUtils.isAbsolute(p))
-      return VirtualFS.resolveVirtual(p);
+    if (this.pathUtils.isAbsolute(pathString))
+      return VirtualFS.resolveVirtual(pathString);
     const resolvedRoot = VirtualFS.resolveVirtual(this.baseFs.resolve(PortablePath.dot));
-    const resolvedP = VirtualFS.resolveVirtual(this.baseFs.resolve(p));
+    const resolvedP = VirtualFS.resolveVirtual(this.baseFs.resolve(pathString));
     return ppath.relative(resolvedRoot, resolvedP) || PortablePath.dot;
   }
   mapFromBase(p) {

I expect the file to be re-generated every time after running yarn add/remove, the exact lines to be patched should change.

This is a very temporary solution and this issue requires actual fix to @yarnpkg/fslib.

PR hints

For those who care enough to go for a PR, you may start here.

https://github.com/yarnpkg/berry/blob/554257087edb4a103633e808253323fb9a21250d/packages/yarnpkg-fslib/sources/VirtualFS.ts#L106

https://github.com/yarnpkg/berry/blob/554257087edb4a103633e808253323fb9a21250d/packages/yarnpkg-fslib/sources/path.ts#L9-L10

+14
vicary on Mar 13, 2022

Still reproducible with the latest yarn 4.0.0-rc.6 from sources and node 18.2.0:

➜  web git:(main) ✗ yarn --version
4.0.0-rc.6.git.20220525.hash-d1961ee
➜  web git:(main) ✗ node
Welcome to Node.js v18.2.0.
Type ".help" for more information.
> const fs = require('fs');
> require('./.pnp.cjs').setup();
> const bufdir = Buffer.from('.');
> fs.readdir(bufdir, { encoding: 'buffer' }, x => console.log(x));
> TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Buffer
    at new NodeError                      (node:internal/errors:377:5)
    at validateString                     (node:internal/validators:119:11)
    at Object.isAbsolute                  (node:path:1157:5)
    at VirtualFS.mapToBase                (.../web/.pnp.cjs:8397:24)
    at VirtualFS.readdirPromise           (.../web/.pnp.cjs:8279:44)
    at PosixFS.readdirPromise             (.../web/.pnp.cjs:8279:24)
    at URLFS.readdirPromise               (.../web/.pnp.cjs:8279:24)
    at                                     .../web/.pnp.cjs:9388:20
    at process.processTicksAndRejections  (node:internal/process/task_queues:77:11) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Using the workaround from @vicary, as above, in the meantime (many thanks!)

Not sure why this issue is labelled as unreproducible though…?

+9
osg74 on May 29, 2022

I just encountered this issue when doing rmSync(storagePath, { recursive: true }); where the directory in storagePath has a very extensive subdirectory structure. The same error is not thrown for a different, simpler directory structure. In both cases I provide the path argument as a string, so the error thrown was quite puzzling to me:

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Buffer
    at new NodeError (node:internal/errors:372:5)
    at validateString (node:internal/validators:120:11)
    at Object.isAbsolute (node:path:1157:5)
    at VirtualFS.mapToBase (/srv/users/brebweb/apps/brebweb/backup/.pnp.cjs:4367:24)
    at VirtualFS.rmdirSync (/srv/users/brebweb/apps/brebweb/backup/.pnp.cjs:4216:39)
    at PosixFS.rmdirSync (/srv/users/brebweb/apps/brebweb/backup/.pnp.cjs:4216:24)
    at URLFS.rmdirSync (/srv/users/brebweb/apps/brebweb/backup/.pnp.cjs:4216:24)
    at _rmdirSync (node:internal/fs/rimraf:260:21)
    at rimrafSync (node:internal/fs/rimraf:193:7)
    at node:internal/fs/rimraf:253:9 {
  code: 'ERR_INVALID_ARG_TYPE'
}
+7
trollkotze on Mar 22, 2022

I forked version 3.2.1 with @vicary’s patch that persists the change to .pnp.cjs: troncali/yarn-3.2.1-pnp-patch.

Yarn runs from a compiled release, so the patch has to be compiled into the release. You can do this by running:

yarn set version from sources --repository https://github.com/troncali/yarn-3.2.1-pnp-patch

Happy to create a PR for this, but unsure if this is the best way to fix the bug. If one of the maintainers wants to provide some feedback or direction on this issue I can work on something more permanent.

+5
troncali on Jul 10, 2022

@trollkotze I encountered the same issue as you. When investigating the stack trace, the buffer is introduced by Node where recursing over child items:

https://github.com/nodejs/node/blob/01408a5aa8d4ed4b486bb8e3d9607f3e342c78b4/lib/internal/fs/rimraf.js#L251

Yarn version: 4.0.0-rc.4 Stack trace:

[error] TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Buffer
    at new NodeError (node:internal/errors:377:5)
    at validateString (node:internal/validators:119:11)
    at Object.isAbsolute (node:path:1157:5)
    at VirtualFS.mapToBase (/data/projects/eternalfest/eternalfest.net/.pnp.cjs:23673:24)
    at VirtualFS.rmdirSync (/data/projects/eternalfest/eternalfest.net/.pnp.cjs:23522:39)
    at PosixFS.rmdirSync (/data/projects/eternalfest/eternalfest.net/.pnp.cjs:23522:24)
    at URLFS.rmdirSync (/data/projects/eternalfest/eternalfest.net/.pnp.cjs:23522:24)
    at _rmdirSync (node:internal/fs/rimraf:260:21)
    at rimrafSync (node:internal/fs/rimraf:193:7)
    at node:internal/fs/rimraf:253:9
+4
demurgos on May 10, 2022

I’m still having this issue on the latest yarn 3.2.4

This fix isn’t in v3.2.4, it hasn’t been released in a stable version yet. We’ll make a new stable release with the fix soon but until then you can use a canary build

yarn set version canary

or build it from source

yarn set version --branch "merceyz/release/3.3.0"
+2
merceyz on Nov 7, 2022
Read more comments on GitHub
← berry: [Bug] Extremely slow `yarn run xxxx` commands on large monorepo
berry: [Bug] `fs` patch doesn't handle `{ bigint: true }` →