Skip to content

Commit

Permalink
Merge pull request #606 from sbillig/downcasterror-crash
Browse files Browse the repository at this point in the history
Fix crash in DowncastError Display impl, remove String member
  • Loading branch information
kjvalencik authored Oct 6, 2020
2 parents a177081 + e5b8154 commit 32f3276
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 9 deletions.
12 changes: 3 additions & 9 deletions src/handle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ impl<'a, T: Managed + 'a> Handle<'a, T> {
pub struct DowncastError<F: Value, T: Value> {
phantom_from: PhantomData<F>,
phantom_to: PhantomData<T>,
description: String
}

impl<F: Value, T: Value> Debug for DowncastError<F, T> {
Expand All @@ -66,22 +65,17 @@ impl<F: Value, T: Value> DowncastError<F, T> {
DowncastError {
phantom_from: PhantomData,
phantom_to: PhantomData,
description: format!("failed downcast to {}", T::name())
}
}
}

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

impl<F: Value, T: Value> Error for DowncastError<F, T> {
fn description(&self) -> &str {
&self.description
}
}
impl<F: Value, T: Value> Error for DowncastError<F, T> {}

/// The result of a call to `Handle::downcast()`.
pub type DowncastResult<'a, F, T> = Result<Handle<'a, T>, DowncastError<F, T>>;
Expand All @@ -90,7 +84,7 @@ impl<'a, F: Value, T: Value> JsResultExt<'a, T> for DowncastResult<'a, F, T> {
fn or_throw<'b, C: Context<'b>>(self, cx: &mut C) -> JsResult<'a, T> {
match self {
Ok(v) => Ok(v),
Err(e) => cx.throw_type_error(&e.description)
Err(e) => cx.throw_type_error(&e.to_string())
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/dynamic/lib/functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ describe('JsFunction', function() {
assert.throw(addon.unexpected_throw_and_catch, Error, /^internal error in Neon module: try_catch: unexpected Err\(Throw\) when VM is not in a throwing state$/);
})

it('should be able to stringify a downcast error', function () {
let msg = addon.downcast_error();
assert.strictEqual(msg, "failed to downcast string to number");
});

it('computes the right number of arguments', function() {
assert.equal(addon.num_arguments(), 0);
assert.equal(addon.num_arguments('a'), 1);
Expand Down
9 changes: 9 additions & 0 deletions test/dynamic/native/src/js/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,12 @@ pub fn unexpected_throw_and_catch(mut cx: FunctionContext) -> JsResult<JsValue>
Ok(cx.try_catch(|_| { Err(Throw) })
.unwrap_or_else(|err| err))
}

pub fn downcast_error(mut cx: FunctionContext) -> JsResult<JsString> {
let s = cx.string("hi");
if let Err(e) = s.downcast::<JsNumber>() {
Ok(cx.string(format!("{}", e)))
} else {
panic!()
}
}
1 change: 1 addition & 0 deletions test/dynamic/native/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ register_module!(mut cx, {
cx.export_function("call_and_catch", call_and_catch)?;
cx.export_function("panic_and_catch", panic_and_catch)?;
cx.export_function("unexpected_throw_and_catch", unexpected_throw_and_catch)?;
cx.export_function("downcast_error", downcast_error)?;

cx.export_class::<JsEmitter>("Emitter")?;
cx.export_class::<JsTestEmitter>("TestEmitter")?;
Expand Down
6 changes: 6 additions & 0 deletions test/napi/lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,10 @@ describe('errors', function() {

assert.throws(() => addon.throw_error(msg), msg);
});

it('should be able to stringify a downcast error', function () {
let msg = addon.downcast_error();
assert.strictEqual(msg, "failed to downcast string to number");
});

});
9 changes: 9 additions & 0 deletions test/napi/native/src/js/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,12 @@ pub fn throw_error(mut cx: FunctionContext) -> JsResult<JsUndefined> {

cx.throw_error(msg)
}

pub fn downcast_error(mut cx: FunctionContext) -> JsResult<JsString> {
let s = cx.string("hi");
if let Err(e) = s.downcast::<JsNumber, _>(&mut cx) {
Ok(cx.string(format!("{}", e)))
} else {
panic!()
}
}
1 change: 1 addition & 0 deletions test/napi/native/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ register_module!(|mut cx| {
cx.export_function("new_type_error", new_type_error)?;
cx.export_function("new_range_error", new_range_error)?;
cx.export_function("throw_error", throw_error)?;
cx.export_function("downcast_error", downcast_error)?;

cx.export_function("panic", panic)?;
cx.export_function("panic_after_throw", panic_after_throw)?;
Expand Down

0 comments on commit 32f3276

Please sign in to comment.