Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pnp: esbuild-wasm@0.15.1 panics when bundling a Yarn PnP project #2458

Closed
merceyz opened this issue Aug 11, 2022 · 4 comments
Closed

pnp: esbuild-wasm@0.15.1 panics when bundling a Yarn PnP project #2458

merceyz opened this issue Aug 11, 2022 · 4 comments

Comments

@merceyz
Copy link

merceyz commented Aug 11, 2022

Describe the bug

When using esbuild-wasm@0.15.1 to bundle a Yarn PnP project it panics with the following error:

⬥ [VERBOSE] Resolving import "./foo.js" in directory "/foo" of type "entry-point"

  Read 21 entries for directory "/"
  The file "/foo/package.json" exists
  The file "/foo/.pnp.cjs" exists
    Extracted JSON data from "/foo/.pnp.cjs"
  Read 10 entries for directory "/foo"
  Read 10 entries for directory "/foo"
  No "browser" map found in directory "/foo"
  Attempting to load "/foo/foo.js" as a file
    Checking for file "foo.js"
    Found file "foo.js"
  Read 10 entries for directory "/foo"
  Primary path is "/foo/foo.js" in namespace "file"

✘ [ERROR] panic: runtime error: invalid memory address or nil pointer dereference (while parsing "foo.js")

  debug.Stack (runtime/debug/stack.go:24)
  helpers.PrettyPrintedStack (internal/helpers/stack.go:9)
  bundler.parseFile.func1 (internal/bundler/bundler.go:178)
  panic (runtime/panic.go:884)
  os.OpenFile (os/file.go:341)
  os.Open (os/file.go:317)
  fs.(*realFS).readdir (internal/fs/fs_real.go:343)
  fs.(*realFS).ReadDirectory (internal/fs/fs_real.go:146)
  fs.(*zipFS).ReadDirectory (internal/fs/fs_zip.go:175)
  resolver.resolverQuery.loadAsFile (internal/resolver/resolver.go:1272)
  resolver.resolverQuery.loadAsFileOrDirectory (internal/resolver/resolver.go:1503)
  resolver.resolverQuery.resolveWithoutSymlinks (internal/resolver/resolver.go:732)
  resolver.(*resolver).Resolve (internal/resolver/resolver.go:412)
  bundler.RunOnResolvePlugins (internal/bundler/bundler.go:812)
  bundler.parseFile (internal/bundler/bundler.go:406)
  bundler.(*scanner).maybeParseFile (internal/bundler/bundler.go:1257)

1 error

Ref yarnpkg/berry#4732

To Reproduce

docker run --rm -it node:16.16.0 bash
mkdir foo
cd foo
yarn init -2
yarn set version 3.2.2
yarn add esbuild-wasm@0.15.1 lodash
printf "require('lodash')" > foo.js
yarn esbuild --bundle --log-level=verbose ./foo.js
@evanw
Copy link
Owner

evanw commented Aug 11, 2022

I'm looking into this. As far as I can tell, the problem is likely the result of the interaction between Go's file system implementation (which uses require('fs')) and Yarn's fs shim (which replaces require('fs')). I'm still trying to figure out exactly what's happening. But that involves debugging Go itself, which has been slow going.

@evanw
Copy link
Owner

evanw commented Aug 11, 2022

I believe the problem is caused by yarnpkg/berry#374, which introduced file descriptors with the high bit set. Go's file descriptor internals does not like file descriptors with the high bit set. They turn into null pointers internally, which then cause issues like this. This makes sense because the open syscall defines file descriptors as non-negative integers, with negative integers reserved for error conditions:

On success, open(), openat(), and creat() return the new file descriptor (a nonnegative integer). On error, -1 is returned and errno is set to indicate the error.

Since Yarn is returning invalid values as file descriptors without indicating an error, this seems like a problem with Yarn, not with esbuild.

@arcanis
Copy link

arcanis commented Aug 11, 2022

I dig a bit and, while the high bit set is a problem, I think the true cause lies elsewhere - specifically in how Go implements the readdir syscall in JS, and in particular the fstat(p).isDirectory() check.

I'm not sure why they do this check rather than let the filesystem decide whether the call is valid, but it highlights a problem we had a few years ago: zip archives being both files and directories, it's not clear whether stat calls should report them as file or directories.

But since we had to pick a side, and we wanted to avoid third-party packages to accidentally recurse within zip files (which would have been slow), we went with representing them as files, except that they accept any directory syscall, and their file contents are also files.

Anyway, the readdir call fails the isDirectory check when performed on zip archives, which cause EINVAL to be generated ... I'm not sure how to fix that in a clean way ... :-/

@merceyz
Copy link
Author

merceyz commented Aug 12, 2022

Since Yarn is returning invalid values as file descriptors without indicating an error, this seems like a problem with Yarn, not with esbuild.
@evanw

Indeed, fixed upstream in yarnpkg/berry#4737, thanks for looking into this!

Anyway, the readdir call fails the isDirectory check when performed on zip archives, which cause EINVAL to be generated ... I'm not sure how to fix that in a clean way ... :-/
@arcanis

Looks like @evanw was able to fix that in 1f17c30.

@merceyz merceyz closed this as completed Aug 12, 2022
@merceyz merceyz changed the title esbuild-wasm@0.15.1 panics when bundling a Yarn PnP project pnp: esbuild-wasm@0.15.1 panics when bundling a Yarn PnP project Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants