Skip to content

feat: expose host print function to Node.js API#55

Open
simongdavies wants to merge 1 commit into
hyperlight-dev:mainfrom
simongdavies:expose-host-print-to-node-js
Open

feat: expose host print function to Node.js API#55
simongdavies wants to merge 1 commit into
hyperlight-dev:mainfrom
simongdavies:expose-host-print-to-node-js

Conversation

@simongdavies
Copy link
Copy Markdown
Member

Add setHostPrintFn() to SandboxBuilder allowing Node.js callers to receive guest console.log/print output via a callback.

  • New setHostPrintFn(callback) method on SandboxBuilder (NAPI layer)
  • Uses ThreadsafeFunction in blocking mode for synchronous print semantics
  • Supports method chaining (returns this)
  • Added to lib.js sync wrapper list for error code extraction
  • 4 new vitest tests: chaining, single log, multiple logs, consumed error
  • index.d.ts auto-generated by napi build with correct TypeScript types

@simongdavies simongdavies added the kind/enhancement New feature or improvement label Mar 18, 2026
@simongdavies simongdavies force-pushed the expose-host-print-to-node-js branch from 54b4d31 to 0e553bc Compare March 18, 2026 19:54
@simongdavies simongdavies requested a review from Copilot March 26, 2026 13:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR exposes guest console.log/print output to Node.js consumers by adding a setHostPrintFn() callback hook on SandboxBuilder, wiring it through the NAPI layer and JS error-enrichment wrapper, and validating behavior via new Vitest tests.

Changes:

  • Add SandboxBuilder.setHostPrintFn() to the Rust NAPI layer using a blocking ThreadsafeFunction<String> to preserve synchronous print semantics.
  • Add a custom lib.js wrapper for setHostPrintFn that catches exceptions thrown by the user’s callback to avoid unhandled errors.
  • Add Vitest coverage for chaining, single/multi log delivery, callback replacement, callback-throw resilience, and consumed-builder errors.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/js-host-api/src/lib.rs Adds the NAPI set_host_print_fn method that registers a host print callback into the underlying sandbox builder.
src/js-host-api/lib.js Wraps setHostPrintFn to catch callback exceptions and keep error-code enrichment behavior consistent.
src/js-host-api/tests/sandbox.test.js Adds tests ensuring host print callback semantics and builder lifecycle behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js-host-api/lib.js
Comment thread src/js-host-api/lib.js
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js-host-api/lib.js
Comment thread src/js-host-api/tests/sandbox.test.js
Comment thread src/js-host-api/src/lib.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js-host-api/src/lib.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js-host-api/tests/sandbox.test.js
Comment thread src/js-host-api/tests/sandbox.test.js Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js-host-api/tests/sandbox.test.js
Comment thread src/js-host-api/lib.js Outdated
Copy link
Copy Markdown
Contributor

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

LGTM, just a small comment, and some thoughts triggered by your code and comments (I think we have a bug in the exisitng host function code)

Comment thread src/js-host-api/lib.js Outdated
Comment on lines +225 to +229
Promise.resolve()
.then(() => callback(msg))
.catch((e) => {
console.error('Host print callback threw:', e);
});
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right the microtask deferral was unnecessary. Fixed in c48ac5c: the callback is now invoked synchronously inline in a try/catch. No Promise wrapping.

// `restore()`, `unload()`) will deadlock. Keep print callbacks
// simple — logging only.
let print_fn = move |msg: String| -> i32 {
let status = callback.call(msg, ThreadsafeFunctionCallMode::Blocking);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be fair, reading the nodejs documentation
https://nodejs.org/api/n-api.html#napi-threadsafe-function-call-mode

A value to be given to napi_call_threadsafe_function() to indicate whether the call should block whenever the queue associated with the thread-safe function is full.

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

napi_call_threadsafe_function() accepts a parameter which controls whether the API behaves blockingly. If set to napi_tsfn_nonblocking, the API behaves non-blockingly, returning napi_queue_full if the queue was full, preventing data from being successfully added to the queue. If set to napi_tsfn_blocking, the API blocks until space becomes available in the queue. napi_call_threadsafe_function() never blocks if the thread-safe function was created with a maximum queue size of 0.

So I think in the host function we are using non-blocking, but we are not checking for the napi_queue_full status.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. Fixed in c48ac5c host function TSFN calls now use Blocking mode too (consistent with print). This ensures calls are always queued rather than silently dropped when the queue is full.

Regarding the broader reentrancy concern you raised: we've added runtime deadlock detection in d2dc25a. If a host callback tries to call back into the sandbox while guest code is executing, it returns ERR_REENTRANT instead of hanging. Filed #192 to track properly separating host function dispatch from the sandbox lock (like core hyperlight does with its separate FunctionRegistry mutex).

Add setHostPrintFn() to SandboxBuilder allowing Node.js callers to
receive guest console.log/print output via a callback.

- setHostPrintFn(callback) on SandboxBuilder (NAPI layer)
- Uses ThreadsafeFunction<String> in Blocking mode
- Callback invoked synchronously in try/catch (no microtask deferral)
- TypeError thrown for non-function arguments
- Host function TSFN calls switched to Blocking mode for consistency
- Runtime deadlock detection (ERR_REENTRANT) when host callbacks
  attempt to call back into the sandbox while guest code is executing
- Vitest coverage for chaining, log delivery, error handling, consumed state

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
@simongdavies simongdavies force-pushed the expose-host-print-to-node-js branch from d2dc25a to ceea3af Compare June 4, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement New feature or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants