Skip to content

fix: runtime JS error that crashes GetPackageJSON#48293

Merged
codebytere merged 1 commit intoelectron:mainfrom
indutny-signal:fix/asar-override-crash
Sep 30, 2025
Merged

fix: runtime JS error that crashes GetPackageJSON#48293
codebytere merged 1 commit intoelectron:mainfrom
indutny-signal:fix/asar-override-crash

Conversation

@indutny-signal
Copy link
Copy Markdown
Contributor

@indutny-signal indutny-signal commented Sep 10, 2025 •
edited
Loading

Description of Change

We overriden the GetPackageJSON in Node.js to let us read files straight from the ASAR file instead of disk. The override works by providing a JS method with the limitation that it should not throw a runtime error. However, this invariant was accidentally violated by asar.splitPath that sometimes contrary to its' TypeScript definition returned false.

Checklist

cc @zcbenz @nikwen

Example of crash stack

0: libsystem_kernel.dylib +0x[REDACTED]
1: libsystem_c.dylib +0x0000000000078a38
2: node::OnFatalError(char const*, char const*)+0xf8 (node_errors.cc:593)
3: v8::Utils::ReportApiFailure(char const*, char const*)+0x4c (api.cc:272)
4: node::modules::BindingData::GetPackageJSON(node::Realm*, std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>>, node::modules::BindingData::ErrorContext*)+0x15f8 (v8-local-handle.h:770)
5: node::modules::BindingData::TraverseParent(node::Realm*, std::__Cr::__fs::filesystem::path const&)+0x460 (node_modules.cc:396)
6: node::modules::BindingData::GetNearestParentPackageJSON(v8::FunctionCallbackInfo<v8::Value> const&)+0x270 (node_modules.cc:432)

Release Notes

Notes: fix: runtime JS error that crashes GetPackageJSON

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Sep 10, 2025
@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 10, 2025 •
edited
Loading

FWIW, the type definition for asar.splitPath is

electron/typings/internal-ambient.d.ts

Lines 91 to 97 in f702327

splitPath(path: string): {
isAsar: false;
} | {
isAsar: true;
asarPath: string;
filePath: string;
};

@deepak1556 deepak1556 added semver/patch backwards-compatible bug fixes target/37-x-y PR should also be added to the "37-x-y" branch. target/38-x-y PR should also be added to the "38-x-y" branch. target/39-x-y PR should also be added to the "39-x-y" branch. labels Sep 10, 2025
deepak1556
deepak1556 approved these changes Sep 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Sep 11, 2025
@jkleinsc
Copy link
Copy Markdown
Member

jkleinsc commented Sep 15, 2025

@indutny-signal can you rebase your PR to pick up a fix for the compilation errors?

@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 15, 2025

Force pushed! Signed the commit while I was there. Thank you!

@jkleinsc jkleinsc mentioned this pull request Sep 17, 2025
Merged
3 tasks
@jkleinsc
Copy link
Copy Markdown
Member

jkleinsc commented Sep 18, 2025

@indutny-signal sorry to ask - can you rebase again to pull in #48345? I tested this fix with your branch and it fixes the build issue seen here.

@indutny-signal
fix: runtime JS error that crashes GetPackageJSON
77bb87b 
We overriden the `GetPackageJSON` in Node.js to let us read files
straight from the ASAR file instead of disk. The override works by
providing a JS method with the limitation that it should not throw a
runtime error. However, this invariant was accidentally violated by
`asar.splitPath` that sometimes contrary to its' TypeScript definition
returned `false`.
@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 18, 2025

@jkleinsc force pushed! np!

@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 25, 2025

@jkleinsc no rush with reviewing it, but let me know if you want me to rebase again!

@codebytere
Copy link
Copy Markdown
Member

codebytere commented Sep 25, 2025

@indutny-signal reran ci and will merge when green!

@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 25, 2025

Looks like it is still not green, but I think the test is flaky?

@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 30, 2025

Should we try once more? 😅

@codebytere codebytere merged commit 6f9cd71 into electron:main Sep 30, 2025
324 of 337 checks passed
@release-clerk
Copy link
Copy Markdown

release-clerk bot commented Sep 30, 2025

Release Notes Persisted

fix: runtime JS error that crashes GetPackageJSON

This was referenced Sep 30, 2025
Merged
Merged
Merged
@trop
Copy link
Copy Markdown
Contributor

trop bot commented Sep 30, 2025

I have automatically backported this PR to "37-x-y", please check out #48423

@trop trop bot added in-flight/37-x-y and removed target/37-x-y PR should also be added to the "37-x-y" branch. labels Sep 30, 2025
@trop
Copy link
Copy Markdown
Contributor

trop bot commented Sep 30, 2025

I have automatically backported this PR to "38-x-y", please check out #48424

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Sep 30, 2025

I have automatically backported this PR to "39-x-y", please check out #48425

@trop trop bot added in-flight/38-x-y in-flight/39-x-y and removed target/38-x-y PR should also be added to the "38-x-y" branch. target/39-x-y PR should also be added to the "39-x-y" branch. labels Sep 30, 2025
@indutny-signal
Copy link
Copy Markdown
Contributor Author

indutny-signal commented Sep 30, 2025

Aww, thank you!

@indutny-signal indutny-signal deleted the fix/asar-override-crash branch September 30, 2025 16:46
@trop trop bot added merged/39-x-y PR was merged to the "39-x-y" branch. merged/37-x-y PR was merged to the "37-x-y" branch. merged/38-x-y PR was merged to the "38-x-y" branch. and removed in-flight/39-x-y in-flight/37-x-y in-flight/38-x-y labels Oct 2, 2025
TheCommieAxolotl pushed a commit to TheCommieAxolotl/electron that referenced this pull request Nov 2, 2025
@indutny-signal @TheCommieAxolotl
fix: runtime JS error that crashes GetPackageJSON (electron#48293)
6c10725 
We overriden the `GetPackageJSON` in Node.js to let us read files
straight from the ASAR file instead of disk. The override works by
providing a JS method with the limitation that it should not throw a
runtime error. However, this invariant was accidentally violated by
`asar.splitPath` that sometimes contrary to its' TypeScript definition
returned `false`.
nilayarya pushed a commit to nilayarya/electron that referenced this pull request Nov 21, 2025
@indutny-signal
fix: runtime JS error that crashes GetPackageJSON (electron#48293)
238197a 
We overriden the `GetPackageJSON` in Node.js to let us read files
straight from the ASAR file instead of disk. The override works by
providing a JS method with the limitation that it should not throw a
runtime error. However, this invariant was accidentally violated by
`asar.splitPath` that sometimes contrary to its' TypeScript definition
returned `false`.
nilayarya added a commit to nilayarya/electron that referenced this pull request Nov 21, 2025
@nilayarya
fix: runtime JS error that crashes GetPackageJSON (electron#48293)
946f153 
We overriden the `GetPackageJSON` in Node.js to let us read files
straight from the ASAR file instead of disk. The override works by
providing a JS method with the limitation that it should not throw a
runtime error. However, this invariant was accidentally violated by
`asar.splitPath` that sometimes contrary to its' TypeScript definition
returned `false`.
nilayarya added a commit to nilayarya/electron that referenced this pull request Nov 21, 2025
@nilayarya
fix: runtime JS error that crashes GetPackageJSON (electron#48293)
8500152 
We overriden the `GetPackageJSON` in Node.js to let us read files
straight from the ASAR file instead of disk. The override works by
providing a JS method with the limitation that it should not throw a
runtime error. However, this invariant was accidentally violated by
`asar.splitPath` that sometimes contrary to its' TypeScript definition
returned `false`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deepak1556 deepak1556 deepak1556 approved these changes

@codebytere codebytere codebytere approved these changes

Assignees

No one assigned

Labels

merged/37-x-y PR was merged to the "37-x-y" branch. merged/38-x-y PR was merged to the "38-x-y" branch. merged/39-x-y PR was merged to the "39-x-y" branch. semver/patch backwards-compatible bug fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@indutny-signal @jkleinsc @codebytere @deepak1556