extism
[WIP] feat: migrate to rquickjs from quickjs-wasm-rs
What is this PR
Resolves #87 by replacing quickjs-wasm-rs with rquickjs
Work to be done
- [x] Replaces the globals.rs initialization with rquickjs
- [x] Migrate the lib.rs file to use rquickjs
- [x] Migrate the init function
- [x] Migrate the export_names functions
- [x] Migrate the "private" invoke function
- [x] Migrate the arg functions
- [x] Migrate the public __invoke functions
- [ ] Verify all of the examples build and run
Note: I'm working on this slowly but wanted to raise this draft PR for visibility and per my reply here
Just wanted to leave an update that I'm still working on this. Had to take a break for a little bit, but have started diving back into it recently.
Note: as of now I've got all of the logic moved to rquickjs and it compiles. Need to do a little code clean up and then run the examples to make sure it's all working. I'm sure a few issues will crop up during that time, but it is at least compiling for now!
@mdarrik that's awesome! Sorry i seemed to miss your last update until just now. I'll take a look at this. Also, we can help you get it over the finish line in terms of fixing docs, examples, etc.
It seems we're failing to load the PRELUDE:
thread '<unnamed>' panicked at crates/core/src/lib.rs:21:9:
Error converting from js 'undefined' into type 'object'
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error: the `wizer.initialize` function trapped
This is happening here: https://github.com/extism/js-pdk/blob/main/crates/core/src/globals.rs#L43
I'm not entirely sure where in the prelude bundle it's encountering this undefined object. It could be one of the injected globals we have. Maybe some of our CJS hacks need to be backed out if we can use ESM.
How can we get the line number of the JS failure?
Darn. Was hoping it would be a bit closer. But was definitely not 100% confident that there weren't issues.
How can we get the line number of the JS failure?
It should be including the stack trace for lines thrown from JS. At least if I'm interpreting this rquickjs issue correctly, so I'm not sure what's missing there. I'll double check what Javvy is doing since that was the template I was using for this.
Looked into it a bit more. Figured out the issue.
It was the define_host_functions code. Was a case issue in getting the Host object there (called it "host" instead of "Host".
It's at least compiling JS files now (tested with the "simple" example in the examples folder). It looks like there might be something up with the logic for actually calling the functions though. That part was the hardest to move, so I'm not fully surprised that it's not working.
Okay I'll see if I can diagnose what could be the problem there. The calling code is pretty tricky.
Just to leave an update, I was able to come back to this finally and have figured out the issue. There are some panics when converting some of the output values in the _invoke_[type] functions. I'm working on fixing those over the next few days. I've been able to run the example/simple.js file and get the correct console output.
As another follow up, the simple example is working successfully now.
> extism -v call out.wasm greet --wasi --input "world" --log-level trace --allow-host "*" --wasi --config "last_name=Manuel"
2024/08/28 23:59:14 Adding wasm file to manifest: out.wasm
2024/08/28 23:59:14 Adding allowed hosts: [*]
2024/08/28 23:59:14 Adding config key last_name=Manuel
2024/08/28 23:59:14 Creating plugin
2024/08/28 23:59:15 No runtime detected
2024/08/28 23:59:15 Got 5 bytes of input data
2024/08/28 23:59:15 Calling greet
2024/08/28 23:59:15 Calling function : greet
2024/08/28 23:59:15 HII
2024/08/28 23:59:15 Call returned 0 bytes
I started to get the Kitchen Sink example working, and it seems like rquickjs is struggling with calling JSON.parse, so I'm looking into that now. My guess is a feature flag or similar needs to be enabled. Error information is below:
Command: extism -v call out.wasm greet --wasi --input "world" --log-level trace --allow-host "*" --wasi --config "last_name=Manuel"
Response:
Error: Encountered an unknown error in call to Extism plugin function greet
Returned non-zero exit code: 4294967293
Okay. I feel silly. JSON.parse is working fine, I was just looking at the wrong thing. I've figured out that the real issue is with Var.get and Var.set. I'll dive into those global functions to see why they aren't quite working.
The gist is that in the Kitchen Sink example, Var.get("input") is returning the string "input" instead of the argument to the function.
Here's a bunch of additional console.logs in the kitchen sink file.
2024/09/01 00:26:31 Adding wasm file to manifest: out.wasm
2024/09/01 00:26:31 Adding allowed hosts: [postman-echo.com]
2024/09/01 00:26:31 Adding config key last_name=Manuel
2024/09/01 00:26:31 Creating plugin
2024/09/01 00:26:32 No runtime detected
2024/09/01 00:26:32 Got 5 bytes of input data
2024/09/01 00:26:32 Calling greet
2024/09/01 00:26:32 Calling function : greet
2024/09/01 00:26:32 input string: Steve
2024/09/01 00:26:33 body.data !== input. Body Data = input. Input = Steve. varInput = input
Okay, so with the latest updates it's now able to run the following examples successfully:
- simple_js
- kitchen_sink
- react
- bundled
The other 2 examples don't error on any quickjs logic. But they're not as clear how to run them since they don't work like those four.
The error handling might be the last place to really improve on. But it does match what's currently on main if the JS returns a non-zero exit code. It just doesn't include traces if the rquickjs code throws an error at the moment.
(So for example, this code and what's on main will print the same thing in the console)
if (!varInput || true) {
Host.outputString("failed to get var: input");
return -1;
}
2024/09/04 00:07:16 Adding wasm file to manifest: ./examples/kitchen-sink.wasm
2024/09/04 00:07:16 Adding allowed hosts: [postman-echo.com]
2024/09/04 00:07:16 Adding config key last_name=Manuel
2024/09/04 00:07:16 Creating plugin
2024/09/04 00:07:17 No runtime detected
2024/09/04 00:07:17 Got 5 bytes of input data
2024/09/04 00:07:17 Calling greet
2024/09/04 00:07:17 Calling function : greet
Error: Encountered an unknown error in call to Extism plugin function greet
Returned non-zero exit code: 4294967295
cc @bhelx since you've been looking at this PR periodically.
@mdarrik sorry i was out last week. Glad to see this progress! I'll take a look at those two examples today.
Those two examples shouldn't cover any different ground from the others. so no need to test them if they aren't being run in the CI.
there are some windows build issues when trying to compile rquickjs. unsure if it's this git patch error, the corrupt tar archive error, or both:
warning: the `wasm32-wasi` target is being renamed to `wasm32-wasip1` and the `wasm32-wasi` target will be removed from nightly in October 2024 and removed from stable Rust in January 2025
error: failed to run custom build command for `rquickjs-sys v0.6.2`
Caused by:
process didn't exit successfully: `D:\a\js-pdk\js-pdk\target\release\build\rquickjs-sys-eddf0bc5a3c43ac5\build-script-build` (exit code: 101)
--- stdout
cargo:rerun-if-changed=build.rs
cargo:rerun-if-env-changed=CARGO_FEATURE_BINDGEN
cargo:rerun-if-env-changed=CARGO_FEATURE_UPDATE_BINDINGS
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_BYTECODE
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_GC
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_GC_FREE
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_FREE
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_LEAKS
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_MEM
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_OBJECTS
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_ATOMS
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_SHAPES
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_MODULE_RESOLVE
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_PROMISE
cargo:rerun-if-env-changed=CARGO_FEATURE_DUMP_READ_OBJECT
Applying patch patches\error_column_number.patch
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/Makefile b/Makefile
|index 0270a6a..1c78547 100644
|--- a/Makefile
|+++ b/Makefile
--------------------------
No file to patch. Skipping patch.
1 out of 1 hunk ignored
patching file cutils.c
patching file cutils.h
patching file quickjs-atom.h
patching file quickjs-opcode.h
patching file quickjs.c
patching file quickjs.h
The next patch would create the file tests/test_line_column.js,
which already exists! Applying it anyway.
patching file tests/test_line_column.js
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file tests/test_line_column.js.rej
Applying patch patches\get_function_proto.patch
patching file quickjs.c
Hunk #1 succeeded at 2222 (offset 7 lines).
patching file quickjs.h
Applying patch patches\check_stack_overflow.patch
patching file quickjs.c
Hunk #1 succeeded at 1622 (offset 32 lines).
Applying patch patches\infinity_handling.patch
patching file quickjs.c
Hunk #1 succeeded at 10391 (offset 105 lines).
Hunk #2 succeeded at 43407 (offset 317 lines).
Hunk #3 succeeded at 49735 (offset 317 lines).
SDK tar: "D:\\a\\js-pdk\\js-pdk\\target\\wasm32-wasi\\release\\build\\rquickjs-sys-a0878a1076653868\\out\\wasi-sdk\\wasi-sdk-20-0.tar.gz"
Extracting WASI SDK archive "D:\\a\\js-pdk\\js-pdk\\target\\wasm32-wasi\\release\\build\\rquickjs-sys-a0878a1076653868\\out\\wasi-sdk\\wasi-sdk-20-0.tar.gz"
--- stderr
thread 'main' panicked at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\rquickjs-sys-0.6.2\build.rs:76:13:
Unpacking WASI SDK failed: tar: Error opening archive: Unrecognized archive format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
warning: `base64` (lib) generated 1 warning
make: *** [Makefile:18: core] Error 101
My guess is patch is probably a place to start based on the existing issues. e.g. https://github.com/DelSkayn/rquickjs/issues/88
I'll do a test on my windows machine later (need to get make and the other deps installed).
Do you want to mark this as ready for review? i can coordinate some testing with the team.
I can do that. I haven't had a ton of luck getting the Windows build issue solved. I also see there are merge conflicts that I'll resolve soon.
This build script inside rquickjs-sys is failing because the mingw wasi-sdk url it's trying to download does not exist. There was a bug in the wasi-sdk build scripts that added an extra .m to the tarball name, which has since been fixed. However, the fix only applies to version 23+ of wasi-sdk and changes the construction of the release urls entirely. (It used to be {major}.{minor}_{suffix}.tar.gz, and it's now {major}.{minor}-{arch}-{plat}.tar.gz.)
A necessary-but-maybe-not-sufficient fix would involve changing how the wasi-sdk download url is constructed in rquickjs-sys's build.rs. (There may be issues related to patch after that too, as @mdarrik pointed out)
Okay after discussing with the team, going to merge this and release as an RC. give us a little more time to test. though I suspect it's going to be just fine looking at the changes and my initial testing. I'll also create an issue for the windows build problem and a note that windows users may need to pin to the stable version.