-
Notifications
You must be signed in to change notification settings - Fork 5
feat: expose host print function to Node.js API #55
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -207,6 +207,30 @@ for (const method of [ | |||||||||||||
| SandboxBuilder.prototype[method] = wrapSync(orig); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // setHostPrintFn needs a custom wrapper: the user's callback is wrapped in | ||||||||||||||
| // try/catch before it reaches the native layer, because exceptions thrown | ||||||||||||||
| // inside a Blocking ThreadsafeFunction escape as unhandled errors. | ||||||||||||||
| { | ||||||||||||||
| const origSetHostPrintFn = SandboxBuilder.prototype.setHostPrintFn; | ||||||||||||||
| if (!origSetHostPrintFn) { | ||||||||||||||
| throw new Error('Cannot wrap missing method: SandboxBuilder.setHostPrintFn'); | ||||||||||||||
| } | ||||||||||||||
| SandboxBuilder.prototype.setHostPrintFn = wrapSync(function (callback) { | ||||||||||||||
| if (typeof callback !== 'function') { | ||||||||||||||
| // Forward non-function values to the native method for consistent | ||||||||||||||
| // validation errors (the Rust layer rejects non-callable arguments). | ||||||||||||||
| return origSetHostPrintFn.call(this, callback); | ||||||||||||||
|
Comment on lines
+220
to
+222
|
||||||||||||||
| // Forward non-function values to the native method for consistent | |
| // validation errors (the Rust layer rejects non-callable arguments). | |
| return origSetHostPrintFn.call(this, callback); | |
| throw new TypeError( | |
| `SandboxBuilder.setHostPrintFn expects a function, received ${typeof callback}` | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this deferring of a microtask?
The host function call is already running in a different thread and is fire and forget (it doesn't wait for the result), so I don't understand why we need this here.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -386,6 +386,56 @@ impl SandboxBuilderWrapper { | |
| inner: Arc::new(Mutex::new(Some(proto_sandbox))), | ||
| }) | ||
| } | ||
|
|
||
| /// Set a callback that receives guest `console.log` / `print` output. | ||
| /// | ||
| /// Without this, guest print output is silently discarded. The callback | ||
| /// receives each print message as a string. | ||
| /// | ||
| /// If the callback throws, the exception is caught by the JS wrapper | ||
| /// (`lib.js`) and logged to `console.error`. Guest execution continues. | ||
| /// | ||
| /// @param callback - `(message: string) => void` — called for each print | ||
| /// @returns this (for chaining) | ||
| /// @throws If the builder has already been consumed by `build()` | ||
| #[napi] | ||
| pub fn set_host_print_fn( | ||
| &self, | ||
| #[napi(ts_arg_type = "(message: string) => void")] callback: ThreadsafeFunction< | ||
| String, // Rust → JS argument type | ||
| (), // JS return type (void) | ||
| String, // JS → Rust argument type (same — identity mapping) | ||
| Status, // Error status type | ||
| false, // Not CallerHandled (napi manages errors) | ||
| false, // Not accepting unknown return types | ||
| >, | ||
| ) -> napi::Result<&Self> { | ||
| self.with_inner(|b| { | ||
| // Blocking mode ensures the TSFN dispatch completes before the | ||
| // native call returns, but the JS wrapper defers the user callback | ||
| // via a Promise microtask — so user code may run after guest | ||
| // execution resumes. From the guest's perspective, print is | ||
| // effectively fire-and-forget with no return value to await. | ||
| // Unlike host functions (which use NonBlocking + oneshot channel | ||
| // for async Promise resolution), print has no result path. | ||
| // | ||
| // **Reentrancy note:** The print callback ultimately runs while | ||
| // the sandbox Mutex is held (inside `call_handler`'s | ||
| // `spawn_blocking`). Calling Hyperlight APIs that acquire the | ||
| // same lock from within the callback (e.g. `snapshot()`, | ||
| // `restore()`, `unload()`) will deadlock. Keep print callbacks | ||
| // simple — logging only. | ||
| let print_fn = move |msg: String| -> i32 { | ||
| let status = callback.call(msg, ThreadsafeFunctionCallMode::Blocking); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be fair, reading the nodejs documentation
IIUC, this is not blocking for the function to finish executing, but for the call to be queued, I don't know what happens in non-blocking mode. I think we should use blocking mode in the host function calls as well (and wait for the result). From here https://nodejs.org/api/n-api.html#calling-a-thread-safe-function
So I think in the host function we are using non-blocking, but we are not checking for the napi_queue_full status. |
||
| if status == Status::Ok { | ||
| 0 | ||
| } else { | ||
| -1 | ||
| } | ||
| }; | ||
| b.with_host_print_fn(print_fn.into()) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // ── ProtoJSSandbox ─────────────────────────────────────────────────── | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| // Basic sandbox functionality tests | ||
| import { describe, it, expect, beforeEach } from 'vitest'; | ||
| import { describe, it, expect, beforeEach, vi } from 'vitest'; | ||
| import { SandboxBuilder } from '../lib.js'; | ||
| import { expectThrowsWithCode, expectRejectsWithCode } from './test-helpers.js'; | ||
|
|
||
|
|
@@ -308,3 +308,120 @@ describe('Calculator example', () => { | |
| expect(result.result).toBe(4); | ||
| }); | ||
| }); | ||
|
|
||
| // ── Host print function ────────────────────────────────────────────── | ||
|
|
||
| describe('setHostPrintFn', () => { | ||
| it('should support method chaining', () => { | ||
| const builder = new SandboxBuilder(); | ||
| const returned = builder.setHostPrintFn(() => {}); | ||
| expect(returned).toBe(builder); | ||
| }); | ||
|
|
||
| it('should receive console.log output from the guest', async () => { | ||
| const messages = []; | ||
| const builder = new SandboxBuilder().setHostPrintFn((msg) => { | ||
| messages.push(msg); | ||
| }); | ||
| const proto = await builder.build(); | ||
| const sandbox = await proto.loadRuntime(); | ||
| sandbox.addHandler( | ||
| 'handler', | ||
| `function handler(event) { | ||
| console.log("Hello from guest!"); | ||
| return event; | ||
| }` | ||
| ); | ||
| const loaded = await sandbox.getLoadedSandbox(); | ||
| await loaded.callHandler('handler', {}); | ||
| // Flush microtasks — the print wrapper defers via Promise | ||
| await new Promise((r) => setTimeout(r, 0)); | ||
|
|
||
| expect(messages.join('')).toContain('Hello from guest!'); | ||
| }); | ||
|
simongdavies marked this conversation as resolved.
|
||
|
|
||
| it('should receive multiple console.log calls', async () => { | ||
| const messages = []; | ||
| const builder = new SandboxBuilder().setHostPrintFn((msg) => { | ||
| messages.push(msg); | ||
| }); | ||
| const proto = await builder.build(); | ||
| const sandbox = await proto.loadRuntime(); | ||
| sandbox.addHandler( | ||
| 'handler', | ||
| `function handler(event) { | ||
| console.log("first"); | ||
| console.log("second"); | ||
| console.log("third"); | ||
| return event; | ||
| }` | ||
| ); | ||
| const loaded = await sandbox.getLoadedSandbox(); | ||
| await loaded.callHandler('handler', {}); | ||
| // Flush microtasks | ||
| await new Promise((r) => setTimeout(r, 0)); | ||
|
|
||
| const combined = messages.join(''); | ||
| expect(combined).toContain('first'); | ||
| expect(combined).toContain('second'); | ||
| expect(combined).toContain('third'); | ||
| }); | ||
|
|
||
| it('should use the last callback when set multiple times', async () => { | ||
| const firstMessages = []; | ||
| const secondMessages = []; | ||
| const builder = new SandboxBuilder() | ||
| .setHostPrintFn((msg) => firstMessages.push(msg)) | ||
| .setHostPrintFn((msg) => secondMessages.push(msg)); | ||
| const proto = await builder.build(); | ||
| const sandbox = await proto.loadRuntime(); | ||
| sandbox.addHandler( | ||
| 'handler', | ||
| `function handler(event) { | ||
| console.log("which callback?"); | ||
| return event; | ||
| }` | ||
| ); | ||
| const loaded = await sandbox.getLoadedSandbox(); | ||
| await loaded.callHandler('handler', {}); | ||
|
|
||
| expect(firstMessages.length).toBe(0); | ||
| expect(secondMessages.join('')).toContain('which callback?'); | ||
| }); | ||
|
Comment on lines
+385
to
+390
|
||
|
|
||
| it('should continue guest execution when callback throws', async () => { | ||
| const builder = new SandboxBuilder().setHostPrintFn(() => { | ||
| throw new Error('print callback exploded'); | ||
| }); | ||
| const proto = await builder.build(); | ||
| const sandbox = await proto.loadRuntime(); | ||
| sandbox.addHandler( | ||
| 'handler', | ||
| `function handler(event) { | ||
| console.log("this will throw in the callback"); | ||
| return { survived: true }; | ||
| }` | ||
| ); | ||
| const loaded = await sandbox.getLoadedSandbox(); | ||
|
|
||
| // Spy on console.error to suppress noise and verify the error is logged | ||
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); | ||
| try { | ||
| const result = await loaded.callHandler('handler', {}); | ||
| // Flush microtasks — the print wrapper defers via Promise | ||
| await new Promise((r) => setTimeout(r, 0)); | ||
|
|
||
| // The JS wrapper catches the throw — guest continues normally | ||
| expect(result.survived).toBe(true); | ||
| expect(errorSpy).toHaveBeenCalledWith('Host print callback threw:', expect.any(Error)); | ||
| } finally { | ||
| errorSpy.mockRestore(); | ||
| } | ||
| }); | ||
|
simongdavies marked this conversation as resolved.
|
||
|
|
||
| it('should throw CONSUMED after build()', async () => { | ||
| const builder = new SandboxBuilder(); | ||
| await builder.build(); | ||
| expectThrowsWithCode(() => builder.setHostPrintFn(() => {}), 'ERR_CONSUMED'); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.