neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
7.88k stars 282 forks source link

Introducing neon-utils #754

Open That3Percent opened 3 years ago

That3Percent commented 3 years ago

I just want to bring to your attention the newly minted neon-utils crate.

crates.io: https://github.com/edgeandnode/neon-utils GitHub: https://github.com/edgeandnode/neon-utils

First, it may be useful to inspire some improvements to neon's APIs (especially around async). But it's also there to work out some issues with neon (like a segfault dealing with Throw). I'm just putting this out there as an example of how I've come to use neon-bindings and the patterns that arose from that usage. I don't really expect you to use all of this, but I'd be willing to help out roll improvements into neon.

run_async

The focal point of this library is the run_async function which schedules a closure on a worker thread and serializes the resulting data for you to JavaScript, while intelligently dealing with errors, etc without the need for creating a Task. One nice feature here is that the closure can capture and mutate data which is a bit tricky normally because the signature of Task::perform takes &self instead of the preferred &mut self or even better self.

Example:

let cb = cx.argument(0)?;

// Generics for Never specified because this callback happens to be infallible
// and type inference wants to know what kind of Error to use
run_async::<_, _, Never>(cb, || {
    let snapshot = api::take_snapshot();
    let bytes = tree_buf::encode(&snapshot);
    Ok(AsArrayBuffer(bytes))
});

This could be improved further if one could create a Promise that resolved from Rust here, instead of taking a callback. The calling code still has to "promisify" the exported function on the JS side just as it currently does using the Task API.

Throw

The other main point of this library is dealing with some unusual difficulties around Throw. These facts interact in unfortunate ways:

Taking all of these together, getting error handling right (especially in the face of terse documentation) is very difficult. The best thing that I could come up with that makes the situation easier but still not foolproof is in errors.rs which introduces a MaybeThrown type to wrap either an error that can be handled or a Throw. It uses the Terminal trait to do the right thing when the method returns. This is far from perfect because you can still get trapped in some of the above issues... but at least I haven't run into any more segfaults using it. The Terminal trait is helpful for methods like run_async to convert values at the boundary ('u32' to JsNumber, etc) in the obvious way when possible, or sometimes with hints like AsArrayBuffer.

js_object

A useful macro for marshalling data. Example usage:

let name = "name";
js_object!(cx => {
    weight: 0.5,
    utility: &self.utility,
    name: name,
})

It creates a JsObject and converts all values to handles (leaving things which are already handled intact) using the same api that Terminal depends on: IntoHandle

Proxy

Useful when you have some Rust data which needs to be passed around in JavaScript with methods. Right now getting access to the Rust data is not convenient.

Example:

pub class NativeSignatureVerifier for Proxy<SignatureVerifier> {
  method verify(mut cx) {
    // Gives me an Arc<SignatureVerifier> so I can access the Rust data... turning 4 lines into 1.
    // A better API may not be tightly coupled with Arc (eg: returning &SignatureVerifier) but
    // that's a bit difficult because the context lock and borrow lifetime and I end up using
    // run_async for everything anyway so just built in Arc made that easier.
    let this = Proxy::this(cx);
  }
}
kjvalencik commented 3 years ago

@That3Percent Thanks for sharing! There's some pretty interesting stuff in here. I'll need to give it a deeper look when I have a chance.

Firstly, producing a segfault using only safe code should be considered a bug and be reported in an issue. You are correct that a user returning Throw is never correct and it should only be returned from Neon. Your suggestion to add private field so that users can't construct it is most likely the best fix.

run_async

You might find this RFC interesting. The problem with including this in Neon is that it can vary in so many different ways. For example, single threaded, thread pool, async executor, etc.

There are also some ideas for cleaning this up with JsPromise.

If throw or any other API returns NeonResult::Err(Throw) and the error is then handled (eg: calling downcast_or_throw in some function, and then in an outer function trying to handle the error by trying another type... this came up when having an API try to read from either a number or string) even though the error is handled and the code can proceed and return NeonResult::Ok then JS still throws an exception after returning from Rust.

This is intentional and unfortunate. Statically preventing a user from interacting with a VM after an exception is throw is very unergonomic. It requires threading Context and gets very difficult, very fast. In nearly all cases of the N-API backend, Neon will panic if the VM is in a throwing state. Let me know if you aren't encountering this.

Most neon APIs deal with Throw directly - meaning handling error conditions is untennable

Can you further explain? There's a try_catch API if you need to recover from an exception, but in most cases propagating the error is sufficient.

You can't easily mix and match error types because converting a value to an exception to call throw requires a Context... meaning that it can't be used with the ? operator

In my experience, it's best to separate code that interacts with the VM and code that doesn't as much as possible so that code only deals with one or the other. This way you can use ? in normal Rust code with no JavaScript interaction. It gets wrapped with code that reads from Js or writes to it.

It might be helpful to have better examples here.

A should also mention that Neon is not 1.0 and still undergoing some breaking changes (most notably to borrowing buffers), not sure how that might impact your crate. Cheers!