fix(native): graceful loading, portable install, and Termux support - #197
Merged
Conversation
`nativeBinding = loadNativeBinding()` ran at module scope, so `import
'wreq-js'` threw whenever the addon could not be resolved. An import that
throws takes the host process down at startup and leaves an embedder no
way to recover: it cannot catch what a static import does, so its only
defence is to make its own import dynamic and demote the dependency.
That is exactly what our largest consumer did, over five commits:
fix(build): include wreq-js native assets in standalone
fix(codex): avoid startup crash when wreq-js is unavailable
fix(codex): make wreq-js import lazy to prevent startup crash
fix(postinstall): extend native module repair to cover wreq-js for pnpm
fix(infrastructure): move wreq-js to optionalDependencies
It is still costing their users. diegosouzapw/OmniRoute#10171 is our
`Unsupported platform:` string reaching a reporter on native Windows and
WSL2 through a handler that misread it as Android/Termux: the server
announced itself as running, every request returned a bare 500, and the
log stayed empty while they worked through `~/.cache` and
`XDG_CACHE_HOME` for a fault that was ours and nowhere near either.
The addon now resolves on first use through `binding()`, which memoises
the failure as well as the addon so a broken install does not re-run
`require` on every call. Importing the package cannot fail; using it
still reports the same diagnostic, at a point where a caller can catch
it.
`isNativeAvailable()` is the supported way to ask in advance, for hosts
that need to choose a fallback rather than handle a throw.
The regression test runs the built bundle from a temporary directory
outside the repository and outside any node_modules, so neither
`../rust/*.node` nor `@wreq-js/binding-*` resolves, and asserts that the
import still succeeds there.
Since 3.1.0 the published `wreq-js` tarball is JavaScript and nothing
else - 0.66 MB across eight files, with every addon living in a
`@wreq-js/binding-*` package that carries its own `os`, `cpu` and `libc`.
The root `os` and `cpu` fields are left over from the 60 MB tarball that
shipped the binaries inline, and they now do active harm.
npm refuses to install a package whose `os` excludes the running platform
and fails the whole tree with EBADPLATFORM. That verdict does not care
that the tarball is portable JavaScript:
npm error code EBADPLATFORM
npm error notsup Unsupported platform for wreq-js@3.1.0:
wanted {"os":"linux,win32","cpu":"x64,arm64"} (current: {"os":"darwin"...})
`os` listed darwin, linux and win32, so on Android the install of any
dependent died outright. The only way to depend on us there was to make
the dependency optional, which is what our largest consumer did, and
which costs them the dependency everywhere else too.
Platform selection belongs to the binding packages, and it already works
there: npm skips an optional dependency whose platform does not match and
carries on. Removing the fields lets the JavaScript install anywhere,
lets the right addon be selected where one exists, and leaves platforms
without one to the graceful path that now exists for them - the import
resolves, `isNativeAvailable()` answers false, and use reports why.
This is the same layout esbuild and swc use for the same reason.
Discussion #110 asked for Android in March and was told no. The probe in August built the target cleanly, so what was left was the load, and the two things that made it look expensive turn out not to be. The addon carries NEEDED libc++_shared.so, because btls-sys asks cmake for CMAKE_ANDROID_STL_TYPE=c++_shared. That library ships with the NDK rather than with Android, which is the usual reason an NDK-built .so fails to load on a device. Termux resolves it from $PREFIX/lib, and its nodejs package declares TERMUX_PKG_DEPENDS="libc++, openssl, c-ares, libicu, libsqlite, ..." so every Termux that can run Node already has it. The dependency is satisfied by construction on the only Android userland that can load us. The other was testing. termux/termux-docker publishes the Termux userland - Bionic, the $PREFIX layout, pkg - as an image, with an aarch64 tag that matches what we build. The Android SDK is only on the x86_64 runner images, so the probe builds there and reaches the aarch64 image through binfmt. It installs nodejs with pkg, which pulls libc++ the same way a user would, and runs the existing smoke test. Peers ship this target without any of that: rollup builds aarch64-linux-android and then excludes android from its test step. `resolveNativePlatform` gains the target. Node reports platform "android" under Termux, and Bionic is neither glibc nor musl, so it is matched ahead of the linux branches rather than sent through detectLibc(). optionalDependencies is deliberately left alone. `napi pre-publish` rewrites it from the npm/ directories during a release, so the entry appears when the package it names is published in the same run. Adding it by hand ahead of that is what produced the versionless placeholders that broke every `npm ci` on master once 3.1.0 published the names they referred to.
/entrypoint.sh hands off to a login shell, which rebuilds the environment
from Termux profile, so WREQ_PLATFORM_ARCH passed through docker -e never
reached the test:
Error: Set WREQ_TARGET or WREQ_PLATFORM_ARCH to locate the native binding.
Everything before that point worked: pkg installed nodejs 26.4.0 and the
runtime reported "android arm64", which is what the loader keys off.
Export it where the test runs, and check libc++_shared.so is present
while we are in there, since the whole probe turns on that library being
part of a Termux that can run Node.
The probe answered its question. In a real Termux userland, on the addon
this workflow now builds:
android arm64
-rwx------ 1 system system 1374336 .../usr/lib/libc++_shared.so
smoke-ok wreq-js.android-arm64.node profiles=133
libc++_shared.so is in $PREFIX/lib before anything asks for it - not
pulled in by the nodejs package, already part of the base userland - so
the NDK dependency that made this target look expensive costs nothing on
the one Android userland that can load us.
The target moves into the release matrix on the same terms as the others:
built, smoke tested, uploaded, and now reachable through napi.targets,
which the publish job turns into a platform package. It cross-compiles on
ubuntu-24.04 because the Android SDK is only on the x86_64 runner images,
and reaches the aarch64 Termux image through binfmt for the load.
Documented in the installation table and the README.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.