Skip to content
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

Migrated error type to napi error #505

Closed
wants to merge 14 commits into from
25 changes: 19 additions & 6 deletions crates/neon-runtime/src/napi/error.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
use raw::Local;
use raw::{Env, Local};

pub unsafe extern "C" fn throw(_val: Local) { unimplemented!() }
use nodejs_sys as napi;

pub unsafe extern "C" fn new_error(_out: &mut Local, _msg: Local) { unimplemented!() }
pub unsafe extern "C" fn throw(env:Env, error: Local)->bool {
let status=napi::napi_throw(env,error);
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!(status, napi::napi_status::napi_ok);
status==napi::napi_status::napi_ok
Copy link
Member

Choose a reason for hiding this comment

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

#Fix: This should assert napi_ok similar to other methods instead of returning a boolean. The Rust code is not checking the result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's still remove the bool return type from this function as @kjvalencik suggests. Once you've asserted that status is napi_ok it's not helpful to test that comparison a second time and then ignore the result anyway.

}

pub unsafe extern "C" fn new_type_error(_out: &mut Local, _msg: Local) { unimplemented!() }
pub unsafe extern "C" fn new_error(out: &mut Local, env:Env, code:Local, msg:Local)->bool {
let status=napi::napi_create_error(env,code,msg,out);
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
status==napi::napi_status::napi_ok
}

pub unsafe extern "C" fn new_range_error(_out: &mut Local, _msg: Local) { unimplemented!() }
pub unsafe extern "C" fn new_type_error(out: &mut Local, env:Env, code:Local, msg:Local)-> bool{
let status=napi::napi_create_type_error(env,code,msg,out);
status==napi::napi_status::napi_ok
}

pub unsafe extern "C" fn throw_error_from_utf8(_msg: *const u8, _len: i32) { unimplemented!() }
pub unsafe extern "C" fn new_range_error(out: &mut Local, env:Env, code:Local, msg:Local)->bool {
let status=napi::napi_create_range_error(env,code,msg,out);
status==napi::napi_status::napi_ok
}
6 changes: 5 additions & 1 deletion src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,11 @@ pub trait Context<'a>: ContextInternal<'a> {
/// Throws a JS value.
fn throw<'b, T: Value, U>(&mut self, v: Handle<'b, T>) -> NeonResult<U> {
unsafe {
neon_runtime::error::throw(v.to_raw());
#[cfg(feature = "napi-runtime")]
neon_runtime::error::throw(self.env().to_raw(),v.to_raw());

#[cfg(feature = "legacy-runtime")]
neon_runtime::error::throw(v.to_raw())
}
Err(Throw)
}
Expand Down
2 changes: 1 addition & 1 deletion src/handle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl<F: Value, T: Value> DowncastError<F, T> {

impl<F: Value, T: Value> Display for DowncastError<F, T> {
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
write!(f, "{}", self.description())
write!(f, "{}", self.to_string())
}
}

Expand Down
35 changes: 32 additions & 3 deletions src/object/class/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ impl<T: Class> Callback<()> for MethodCallback<T> {
};
let dynamic_callback: fn(CallContext<T>) -> JsResult<JsValue> =
mem::transmute(neon_runtime::fun::get_dynamic_callback(data.to_raw()));
if let Ok(value) = convert_panics(|| { dynamic_callback(cx) }) {

#[cfg(feature = "napi-runtime")]
let result=convert_panics(cx,|cx| { dynamic_callback(cx) });

#[cfg(feature = "legacy-runtime")]
let result=convert_panics(|| { dynamic_callback(cx) });

if let Ok(value) = result {
info.set_return(value);
}
})
Expand Down Expand Up @@ -65,7 +72,12 @@ impl Callback<()> for ConstructorCallCallback {
let data = info.data();
let kernel: fn(CallContext<JsValue>) -> JsResult<JsValue> =
mem::transmute(neon_runtime::class::get_call_kernel(data.to_raw()));
if let Ok(value) = convert_panics(|| { kernel(cx) }) {
#[cfg(feature = "napi-runtime")]
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
let result= convert_panics(cx,|cx| { kernel(cx) });

#[cfg(feature = "legacy-runtime")]
let result= convert_panics(|| { kernel(cx) });
if let Ok(value) =result {
info.set_return(value);
}
})
Expand All @@ -87,7 +99,14 @@ impl<T: Class> Callback<*mut c_void> for AllocateCallback<T> {
let data = info.data();
let kernel: fn(CallContext<JsUndefined>) -> NeonResult<T::Internals> =
mem::transmute(neon_runtime::class::get_allocate_kernel(data.to_raw()));
if let Ok(value) = convert_panics(|| { kernel(cx) }) {

#[cfg(feature = "napi-runtime")]
let result=convert_panics(cx,|cx| { kernel(cx) });

#[cfg(feature = "legacy-runtime")]
let result=convert_panics(|| { kernel(cx) });

if let Ok(value) = result {
let p = Box::into_raw(Box::new(value));
mem::transmute(p)
} else {
Expand All @@ -112,6 +131,16 @@ impl<T: Class> Callback<bool> for ConstructCallback<T> {
let data = info.data();
let kernel: fn(CallContext<T>) -> NeonResult<Option<Handle<JsObject>>> =
mem::transmute(neon_runtime::class::get_construct_kernel(data.to_raw()));
#[cfg(feature = "napi-runtime")]
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
match convert_panics(cx,|cx| { kernel(cx) }) {
Ok(None) => true,
Ok(Some(obj)) => {
info.set_return(obj);
true
}
_ => false
}
#[cfg(feature = "legacy-runtime")]
match convert_panics(|| { kernel(cx) }) {
Ok(None) => true,
Ok(Some(obj)) => {
Expand Down
129 changes: 117 additions & 12 deletions src/types/error.rs
Original file line number Diff line number Diff line change
@@ -1,70 +1,140 @@
//! Types and traits representing JavaScript error values.

use std::panic::{UnwindSafe, catch_unwind};
use std::panic::{catch_unwind, UnwindSafe};

use neon_runtime;
use neon_runtime::raw;

use context::Context;
use result::{NeonResult, Throw};
use types::{Value, Object, Handle, Managed, build};
use types::internal::ValueInternal;
use types::utf8::Utf8;
use types::{build, Handle, Managed, Object, Value};

/// A JS `Error` object.
#[repr(C)]
#[derive(Clone, Copy)]
pub struct JsError(raw::Local);

impl Managed for JsError {
fn to_raw(self) -> raw::Local { self.0 }
fn to_raw(self) -> raw::Local {
self.0
}

fn from_raw(h: raw::Local) -> Self { JsError(h) }
fn from_raw(h: raw::Local) -> Self {
JsError(h)
}
}

impl ValueInternal for JsError {
fn name() -> String { "Error".to_string() }
fn name() -> String {
"Error".to_string()
}

fn is_typeof<Other: Value>(other: Other) -> bool {
unsafe { neon_runtime::tag::is_error(other.to_raw()) }
}
}

impl Value for JsError { }
impl Value for JsError {}

impl Object for JsError { }
impl Object for JsError {}

impl JsError {
/// Creates a direct instance of the [`Error`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Error) class.
pub fn error<'a, C: Context<'a>, S: AsRef<str>>(cx: &mut C, msg: S) -> NeonResult<Handle<'a, JsError>> {
pub fn error<'a, C: Context<'a>, S: AsRef<str>>(
cx: &mut C,
msg: S,
) -> NeonResult<Handle<'a, JsError>> {
#[cfg(feature = "legacy-runtime")]
let msg = cx.string(msg.as_ref());

#[cfg(feature = "napi-runtime")]
let (ptr, len) = if let Some(small) = Utf8::from(msg.as_ref()).into_small() {
Copy link
Member

Choose a reason for hiding this comment

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

Are we able to re-use the JsString code for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JsString will create a struct and then i have to use to_raw on it is not ideal according to me

small.lower()
} else {
return Err(Throw);
};
build(|out| unsafe {
#[cfg(feature = "napi-runtime")]
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
let mut local: raw::Local = std::mem::zeroed();
#[cfg(feature = "napi-runtime")]
neon_runtime::string::new(&mut local, cx.env().to_raw(), ptr, len);
#[cfg(feature = "napi-runtime")]
neon_runtime::error::new_error(out, cx.env().to_raw(), std::ptr::null_mut(), local);
#[cfg(feature = "legacy-runtime")]
neon_runtime::error::new_error(out, msg.to_raw());
true
})
}

/// Creates an instance of the [`TypeError`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/TypeError) class.
pub fn type_error<'a, C: Context<'a>, S: AsRef<str>>(cx: &mut C, msg: S) -> NeonResult<Handle<'a, JsError>> {
pub fn type_error<'a, C: Context<'a>, S: AsRef<str>>(
cx: &mut C,
msg: S,
) -> NeonResult<Handle<'a, JsError>> {
#[cfg(feature = "legacy-runtime")]
let msg = cx.string(msg.as_ref());

#[cfg(feature = "napi-runtime")]
let (ptr, len) = if let Some(small) = Utf8::from(msg.as_ref()).into_small() {
small.lower()
} else {
return Err(Throw);
};
build(|out| unsafe {
#[cfg(feature = "napi-runtime")]
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
let mut local: raw::Local = std::mem::zeroed();
#[cfg(feature = "napi-runtime")]
neon_runtime::string::new(&mut local, cx.env().to_raw(), ptr, len);
#[cfg(feature = "napi-runtime")]
neon_runtime::error::new_type_error(
out,
cx.env().to_raw(),
std::ptr::null_mut(),
local,
);
#[cfg(feature = "legacy-runtime")]
neon_runtime::error::new_type_error(out, msg.to_raw());
true
})
}

/// Creates an instance of the [`RangeError`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/RangeError) class.
pub fn range_error<'a, C: Context<'a>, S: AsRef<str>>(cx: &mut C, msg: S) -> NeonResult<Handle<'a, JsError>> {
pub fn range_error<'a, C: Context<'a>, S: AsRef<str>>(
cx: &mut C,
msg: S,
) -> NeonResult<Handle<'a, JsError>> {
#[cfg(feature = "legacy-runtime")]
let msg = cx.string(msg.as_ref());

#[cfg(feature = "napi-runtime")]
let (ptr, len) = if let Some(small) = Utf8::from(msg.as_ref()).into_small() {
small.lower()
} else {
return Err(Throw);
};
build(|out| unsafe {
#[cfg(feature = "napi-runtime")]
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
let mut local: raw::Local = std::mem::zeroed();
#[cfg(feature = "napi-runtime")]
neon_runtime::string::new(&mut local, cx.env().to_raw(), ptr, len);
#[cfg(feature = "napi-runtime")]
neon_runtime::error::new_range_error(
out,
cx.env().to_raw(),
std::ptr::null_mut(),
local,
);
#[cfg(feature = "legacy-runtime")]
neon_runtime::error::new_range_error(out, msg.to_raw());
true
})
}
}

#[cfg(feature = "legacy-runtime")]
pub(crate) fn convert_panics<T, F: UnwindSafe + FnOnce() -> NeonResult<T>>(f: F) -> NeonResult<T> {
match catch_unwind(|| { f() }) {
match catch_unwind(|| f()) {
Ok(result) => result,
Err(panic) => {
let msg = if let Some(string) = panic.downcast_ref::<String>() {
Expand All @@ -82,3 +152,38 @@ pub(crate) fn convert_panics<T, F: UnwindSafe + FnOnce() -> NeonResult<T>>(f: F)
}
}
}

#[cfg(feature = "napi-runtime")]
pub(crate) fn convert_panics<
'a,
T,
C: Context<'a> + std::panic::UnwindSafe,
F: UnwindSafe + FnOnce(C) -> NeonResult<T>,
>(
cx: C,
f: F,
) -> NeonResult<T> {
let env = cx.env().to_raw();
anshulrgoyal marked this conversation as resolved.
Show resolved Hide resolved
match catch_unwind(move || f(cx)) {
Ok(result) => result,
Err(panic) => {
let msg = if let Some(string) = panic.downcast_ref::<String>() {
format!("internal error in Neon module: {}", string)
} else if let Some(str) = panic.downcast_ref::<&str>() {
format!("internal error in Neon module: {}", str)
} else {
"internal error in Neon module".to_string()
};
println!("{}", msg);
let (data, len) = Utf8::from(&msg[..]).truncate().lower();
unsafe {
let mut local: raw::Local = std::mem::zeroed();
let mut error: raw::Local = std::mem::zeroed();
neon_runtime::string::new(&mut local, env, data, len);
neon_runtime::error::new_error(&mut error, env, std::ptr::null_mut(), local);
neon_runtime::error::throw(env,error);
};
Err(Throw)
}
}
}
8 changes: 7 additions & 1 deletion src/types/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ impl<T: Value> Callback<()> for FunctionCallback<T> {
let data = info.data();
let dynamic_callback: fn(FunctionContext) -> JsResult<T> =
mem::transmute(neon_runtime::fun::get_dynamic_callback(data.to_raw()));
if let Ok(value) = convert_panics(|| { dynamic_callback(cx) }) {
#[cfg(feature = "napi-runtime")]
let result=convert_panics(cx,|cx| { dynamic_callback(cx) });

#[cfg(feature = "legacy-runtime")]
let result=convert_panics(|| { dynamic_callback(cx) });

if let Ok(value) = result{
info.set_return(value);
}
})
Expand Down