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

NodeJS 22 issues/feedbacks tracker #87

Open
robertsLando opened this issue Sep 10, 2024 · 59 comments
Open

NodeJS 22 issues/feedbacks tracker #87

robertsLando opened this issue Sep 10, 2024 · 59 comments

Comments

@robertsLando
Copy link
Member

robertsLando commented Sep 10, 2024

NodeJS 22 support has been added starting from pkg 5.14.0 but many things have changed on NodeJS source and so also on our patch file so it needs some tests. Use this issue to submit your feedbacks and issues

KNOWN ISSUES:

cc @faulpeltz

@robertsLando robertsLando pinned this issue Sep 10, 2024
@robertsLando robertsLando changed the title NodeJS 22 issues tracker NodeJS 22 issues/feedbacks tracker Sep 10, 2024
@faulpeltz
Copy link

I managed to complete a Node 22 build with macos arm64 after removing the android SDK (the change from the node/build repo), and then also removed the iOS simulator cache (which is nearly 50GB)

yao-pkg/pkg-fetch@main...faulpeltz:pkg-fetch:main#diff-a1654f4357833dfc4d0c6a8d39c2e3ee30ceea604c29238846903993434ab384R93

@robertsLando
Copy link
Member Author

@faulpeltz That's super! PR? 🚀

@robertsLando
Copy link
Member Author

I would extend that also on x64 build to prevent any future issue with this :)

@faulpeltz
Copy link

@faulpeltz That's super! PR? 🚀

I'm trying to find out why the x64 macos build on -13 is so fragile, I had it crash or timeout multiple times. But the cleanup seems not necessary for now because it has ~200GB free disk space, and I assume it will imrpove in the future 🤷‍♂️

I really don't want to complain about free Apple build infrastructure, but 14GB of free disk space is a bit of a stretch...

@robertsLando
Copy link
Member Author

I really don't want to complain about free Apple build infrastructure, but 14GB of free disk space is a bit of a stretch...

Yeah definetely 😆 Also because storage nowdays doesn't cost that much, RAM and CPU does...

@cage1618
Copy link

Can I use this release in production now?

@robertsLando
Copy link
Member Author

@cage1618 If you are speaking about nodejs 22 I would say nope, but you can use nodejs 18/20 safely with latest release

@faulpeltz
Copy link

@robertsLando The Node 22 patch applies to the just-released 22.9 version without problems and all builds went through

In this release they disabled Maglev (one of the JIT compilers) because of reported crashes
https://github.com/nodejs/node/releases/tag/v22.9.0

So random crashes might be caused by latent Node 22 issues before 22.9, might make sense to bump the Node 22 version (but not urgent)

@robertsLando
Copy link
Member Author

@faulpeltz thanks for the info, I have an action to automatically create the patch, let me run it 🚀

@robertsLando
Copy link
Member Author

I have created a workflow that automatically check for new nodejs versions: https://github.com/yao-pkg/pkg-fetch/actions/workflows/check-latest-node.yml

@cage1618
Copy link

release 5.15.0 is stable now?

@balisaurus
Copy link

Using 5.15.0 to use latest nodejs 22 doesn't seem to work for my project.
I create a simple test code with a single file writing hello to console:

pkg --debug hello.js --targets node22-win-x64
> [email protected]
(node:27328) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
> [debug] Bytecode of D:\temp\testnode\hello.js is added to queue.
> [debug] Stat info of D:\temp\testnode\hello.js is added to queue.
> [debug] Directory D:\temp\testnode is added to queue.
> [debug] Stat info of D:\temp\testnode is added to queue.
> [debug] Directory D:\temp is added to queue.
> [debug] Stat info of D:\temp is added to queue.
> [debug] Directory D:\ is added to queue.
> [debug] Stat info of D:\ is added to queue.
> [debug] Deleting record file : D:\
> [debug] The file was included as bytecode (no sources)
  D:\temp\testnode\hello.js
> [debug] The file was included as bytecode (no sources)
  D:\temp\testnode\hello.js
> [debug] files & folders deduped =
  hello.js
> [debug] The directory files list was included (1 item)
  D:\temp\testnode
> [debug] The directory files list was included (1 item)
  D:\temp\testnode
> [debug] files & folders deduped =
  testnode
> [debug] The directory files list was included (1 item)
  D:\temp
> [debug] The directory files list was included (1 item)
  D:\temp
> [debug] Targets:
  [
  {
    "nodeRange": "node22",
    "platform": "win",
    "arch": "x64",
    "output": "d:\\temp\\testnode\\hello.exe",
    "forceBuild": false,
    "fabricator": {
      "nodeRange": "node22",
      "platform": "win",
      "arch": "x64",
      "binaryPath": "C:\\Users\\tekno\\.pkg-cache\\v3.5\\fetched-v22.9.0-win-x64"
    },
    "binaryPath": "C:\\Users\\tekno\\.pkg-cache\\v3.5\\fetched-v22.9.0-win-x64"
  }
]

Running it:

hello.exe
------------------------------- virtual file system
C:\snapshot
  testnode                                29 Bytes
    hello.js                              29 Bytes
Total size =  29 Bytes
node:internal/modules/cjs/loader:1254
  throw err;
  ^

Error: Cannot find module 'C:\snapshot\testnode\hello.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1251:15)
    at Function._resolveFilename (pkg/prelude/bootstrap.js:1955:46)
    at Module._load (node:internal/modules/cjs/loader:1077:27)
    at Function.runMain (pkg/prelude/bootstrap.js:1983:12)
    at node:internal/main/run_main_module:30:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v22.9.0

@robertsLando
Copy link
Member Author

@balisaurus does it work with others nodejs version?

@BeneRasche
Copy link

Same problem here:

node:internal/modules/cjs/loader:1254
throw err;
^

Error: Cannot find module 'C:\snapshot\appname\dist\apps\server\main.js'
at Module._resolveFilename (node:internal/modules/cjs/loader:1251:15)
at Function._resolveFilename (pkg/prelude/bootstrap.js:1955:46)
at Module._load (node:internal/modules/cjs/loader:1077:27)
at Function.runMain (pkg/prelude/bootstrap.js:1983:12)
at node:internal/main/run_main_module:30:49 {
code: 'MODULE_NOT_FOUND',
requireStack: []
}

Node.js v22.9.0


works with -t node20-win-x64 but not node22

@faulpeltz
Copy link

Seems that the snapshot fs doesn't work for module loading on Windows in the Node 22 patch

@robertsLando
Copy link
Member Author

@BeneRasche The window issue related to nodejs 22 should be fixed on next release, please be patient. Thanks to @faulpeltz for the patch!

@thisjt
Copy link

thisjt commented Sep 30, 2024

We're waiting for this, right?

@faulpeltz Unfortunately we need to wait a new nodejs 22 release in order to test this as I have no way with current workflows to force build a specific version, I need to implement this

Based on how often nodejs releases, I think it will be either next week or mid October

@robertsLando
Copy link
Member Author

robertsLando commented Sep 30, 2024

@thisjt yep, I'm sorry!

Problem is that actually I cannot release patches for nodejs patches as them would become broke for actual pkg versions as the shas are shipped with npm so updating them would change them and work only for latest vesrsions. I already have a workflow that runs every day at midnight and opens a PR when a new patch is available

@thisjt
Copy link

thisjt commented Oct 7, 2024

@robertsLando nodejs had a release 4 days ago. I think it's time to patch 😬 let's gooo- or does it need to have a v22 release?
https://github.com/nodejs/node/releases/tag/v20.18.0


Oh lol nvm my bad it needs a v22 release

@robertsLando
Copy link
Member Author

robertsLando commented Oct 8, 2024

@thisjt thanks for letting me know! Seems my automated workflows picked up it correctly 4 days ago but the patches do not apply cleanly, let me try to fix this

https://github.com/yao-pkg/pkg-fetch/actions/runs/11171900799/job/31057489528

BTW seems only node 20 has a new release for now

@thisjt
Copy link

thisjt commented Oct 18, 2024

node v22.10 is finally out! https://github.com/nodejs/node/releases/tag/v22.10.0

And with that node v23 is also out, hoping that we will also be supporting this. Not in a hurry tho, we're not sure yet on how stable v23 is.

@dingiyan
Copy link

I often follow this library and hope to keep up with the latest version of Node.js. This morning, I tested the v22 version, but it is not yet available. I hope excellent authors can continue to follow up and look forward to the latest available v22 version. Finally, I would like to thank the contributors of yao-pkg for their hard work

@robertsLando
Copy link
Member Author

New version is coming soon. The average delay I would like to keep between nodejs releases and pkg support for them is around 1 week, it depends on how many breaks there are on patches.

@robertsLando
Copy link
Member Author

pkg 5.16.0 is out now with node 20.18.0 and 22.10.0 support

@BeneRasche
Copy link

pkg-fetch probably needs an update for 22.10 binaries, too?

@robertsLando
Copy link
Member Author

@BeneRasche It's out already!

@BeneRasche
Copy link

@BeneRasche It's out already!

Where?

https://github.com/yao-pkg/pkg-fetch/releases

@robertsLando
Copy link
Member Author

Someone wants to give this a try? #110 🚀

@dingiyan
Copy link

@dingiyan thanks for your detailed issue! Could you tell me if that's just related to 22-win? Are you also able to reproduce it with 22-linux? I think this may be related to:

https://github.com/yao-pkg/pkg-fetch/blob/main/patches/node.v22.10.0.cpp.patch#L344-L346

@faulpeltz what do you think?

There is also a reference to https://github.com/yao-pkg/pkg/blob/main/prelude/bootstrap.js#L1942

But I don't think the issue is there

I tested on Mac, and got same error. I think linux is too.
I think the issue point is node.v22.10.0.cpp.patch

@faulpeltz
Copy link

@dingiyan thanks for your detailed issue! Could you tell me if that's just related to 22-win? Are you also able to reproduce it with 22-linux? I think this may be related to:

https://github.com/yao-pkg/pkg-fetch/blob/main/patches/node.v22.10.0.cpp.patch#L344-L346

@faulpeltz what do you think?

There is also a reference to https://github.com/yao-pkg/pkg/blob/main/prelude/bootstrap.js#L1942

But I don't think the issue is there

I can reproduce it on Linux, and I'm working on a fix

@robertsLando
Copy link
Member Author

Ok will be fixed on next node 22 release 🙏🏼

@robertsLando
Copy link
Member Author

The alternative is to delete binaries from release and redo the release now

@faulpeltz
Copy link

Would it be possible to create a new GH release for each pkg fetch release and keep the new binaries there?

@robertsLando
Copy link
Member Author

Would it be possible to create a new GH release for each pkg fetch release and keep the new binaries there?

That's actually not possible as pkg downloads assets from pkg-fetch GH releases.

@thisjt
Copy link

thisjt commented Oct 22, 2024

Maybe we can add a feature wherein we can specifically target a pkg-fetch release instead of getting the latest one for that specific node version. That way we'll have some flexibility on what pkg-fetch release we can specifically target instead of always getting the latest one.

I'd need to look into how the entire pkg-fetch works as a whole though so I might need a bit of schooling as to why this is, or why this is not possible or viable. Would be nice to have this feature so that we can roll out hotfixes instead of waiting for a new node release every time we need to fix an issue.

@robertsLando
Copy link
Member Author

@dingiyan
Copy link

dingiyan commented Nov 4, 2024

Hello, I view the 22.11.0 versions was released on pkg-fetch, but I got version 22.10.0 when I use pkg build. I test the @yao-pkg/pkg on 5.16.1 and 6.0.1

@robertsLando
Copy link
Member Author

robertsLando commented Nov 4, 2024

@dingiyan Because I triggered the workflow to build nodejs patches but then completely forgot to create a new release (workflow takes 4/5hours to complete). It's out now: 6.1.0

@CodingBear-Git
Copy link

CodingBear-Git commented Nov 21, 2024

Hello! I don't know if I'm writing in the right topic, but let it be here.
When trying to compile an application into an executable file, I had a situation where the application was compiled perfectly for targets: latest-linux-x64 and latest-win-x64. Then on Linux everything started and worked fine, but when trying to run on Win, the application did NOT work and did NOT give any errors.
I built the application like this: first, I built the entire source code with esbuild into 1 file, like this:

{
    entryPoints: ["./src/app.ts"],
    bundle: true,
    platform: "node",
    keepNames: true,
    minify: true,
    outfile: "./dist/app-bundle.js"
}

And then I built the application from this file using pkg like this:

"pkg": {
    "assets": [],
    "ignore": [],
    "outputPath": "dist/bin",
    "targets": [
        "latest-win-x64",
        "latest-linux-x64"
    ]
}

As a result, I found out that it is enough to add the file created with esbuild to assets. Like this:

"assets": [
    "dist/app-bundle.js"
]

After that I rebuilt the application and it works well on both Linux and Win.
With this message I want to say that maybe somewhere in the readme it is worth writing that when using esbuild you need to explicitly specify "assets": ["dist/app-bundle.js" ] in config pkg.

P.S.
At the same time, for targets node20-win-x64 everything was assembled and worked without adding anything to assets.

PPS
@robertsLando Thanks for all the work you do to maintain this package!

@robertsLando
Copy link
Member Author

@CodingBear-Git what is the pkg command you run to package your application? Because it should have bin property set to dist/app-bundle.js

@CodingBear-Git
Copy link

@robertsLando pkg ./dist/app-bundle.js --config package.json

package.json

"pkg": {
    "assets": [],
    "ignore": [],
    "outputPath": "dist/bin",
    "targets": [
      "latest-win-x64",
      "latest-linux-x64",
      "latest-mac-x64"
    ]
  }

But for some reason, with this method, the application did not work on windows.

@CodingBear-Git
Copy link

@robertsLando I made a minimal example that reproduces the problem - https://github.com/CodingBear-Git/test-pkg

@xcjs
Copy link

xcjs commented Nov 23, 2024

@robertsLando pkg ./dist/app-bundle.js --config package.json

package.json

"pkg": {
    "assets": [],
    "ignore": [],
    "outputPath": "dist/bin",
    "targets": [
      "latest-win-x64",
      "latest-linux-x64",
      "latest-mac-x64"
    ]
  }

But for some reason, with this method, the application did not work on windows.

I've had the same issue with Windows builds just now. The binary exits with no error messaging.

@faulpeltz
Copy link

I can reproduce this with the Windows build only, it works when including the sources with "--public" (both with and without bytecode), so something is breaking when the sources are not found, but since the other platforms are working...

@faulpeltz
Copy link

Also, when running pkg on a Windows box the windows build works fine, and building the linux binary on the Windows box also results in a broken binary not working on linux anymore.
So it looks like the bytecode might be incompatible/different depending on the build platform?

@robertsLando
Copy link
Member Author

I've had the same issue with Windows builds just now. The binary exits with no error messaging.

@xcjs Did you tried to add that to assets like @CodingBear-Git did?

So it looks like the bytecode might be incompatible/different depending on the build platform?

@faulpeltz bytecode is always different I mean the sha of bytecode always changes on each run as it's not deterministic. ATM the bytecode is created by https://github.com/yao-pkg/pkg/blob/main/lib/fabricator.ts but sincerely I have no clue what could be broken here

@xcjs
Copy link

xcjs commented Nov 25, 2024

I've had the same issue with Windows builds just now. The binary exits with no error messaging.

@xcjs Did you tried to add that to assets like @CodingBear-Git did?

So it looks like the bytecode might be incompatible/different depending on the build platform?

@faulpeltz bytecode is always different I mean the sha of bytecode always changes on each run as it's not deterministic. ATM the bytecode is created by https://github.com/yao-pkg/pkg/blob/main/lib/fabricator.ts but sincerely I have no clue what could be broken here

I haven't tried to add assets. The same build works with Node 20, though without assets.

@faulpeltz
Copy link

Just FYI, the Github Actions builds for NodeJS on Windows stopped working because the GH runner image (windows-2022) contains a VS version (17.12) which is rejected by the NodeJS build scripts (on purpose, because it causes compile errors, caused by a compiler bug)

MS has acknowledged the bug and a fix is pending:
https://developercommunity.visualstudio.com/t/Regression-in-MSVC-v1712:-Compilation-E/10794016#T-N10797290

nodejs/node#55929

@CodingBear-Git
Copy link

I checked. Same problem on Mac OS.

@faulpeltz
Copy link

@robertsLando
I've tried solving the cross-platform build issue, without much success, here is what I found up to now:

  • the binary failing to do anything (e.g. windows binary built on linux without sources) is caused by Node/V8 failing to load the payload script from bytecode, falling back to an empty script (which doesn't do anything)
  • when loading the bytecode, it will check various hashes/flags to make sure the bytecode was built with the correct node version
  • for Node 22 when loading on e.g. Windows the hash check for kReadOnlySnapshot will fail because the snapshot hash is different on linux/windows/mac
  • disabling the node snapshot does not help, it seems the v8 base snapshot is already different, not sure if that can be turned off
  • disabling the hash check will result in a hard startup segfault/crash
  • when building with source ("--public") the broken bytecode is ignored on load, and it uses the JS code which works

What I'm wondering about if cross-platform works with Node SEA, because internally it seems to use the same APIs to build and load the bytecode, or if it also just falls back to the JS script..

@robertsLando
Copy link
Member Author

@faulpeltz thanks for the detailed explanation!

From what I know SEA doesn't execute bytecode directly, it generates a blob (that contains the source code plus some other bytes) to inject it to the nodejs executable but it's not compiled in bytecode.

I think the only one that could answer to this is @joyeecheung (who implemented node SEA feature) but not sure if he will find the time to reply here.

Another one that could probably have some hints is @igorklopov

@igorklopov
Copy link

IIRC there was a mechanism in pkg that forces generating such a bytecode that is cross-platform (otherwise without the mechanism the bytecode is platform-specific).

@robertsLando
Copy link
Member Author

robertsLando commented Dec 17, 2024

@igorklopov The bytecode generation is handled in https://github.com/yao-pkg/pkg/blob/main/lib/fabricator.ts but I don't think this file received many changes in last years also I don't see any check for cross platform there or in producer

@igorklopov
Copy link

It was i::CpuFeatures::Reinitialize but it was in node v10 therefore unrelated (because v20 works without having Reinitialize in EnableCompilationForSourcelessUse according to reports above)
https://github.com/yao-pkg/pkg-fetch/blob/361ac29084daec85634c0cd04bda7d73c2869cfc/patches/node.v10.24.1.cpp.patch#L35

@robertsLando
Copy link
Member Author

Oh you were speaking about patches so, anyway seems not an issue as we dropped that many patches ago and this only happens on node 22 :(

@faulpeltz
Copy link

In ContextifyScript::New() it does set the V8 flags "lazy" to false and "predictable" to true (this is code added by pkg),
the fabricator just uses vm.Script with "produceCachedData: true".

If there is another mechanism I don't know where it can be found - but maybe this is either just a result of a breaking change in Node 22 (or the new V8 version it uses) or maybe some new flags need to be set

When looking at the produced code cache blob on Linux and Windows, the generated code is exactly the same for a small but non-working example, but the hashes for the read-only snapshots are different - if someone has an idea where these are coming from or where they are generated during build, maybe I can find out whats different

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

10 participants