Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upThrow type error on converting non objects to sequence #238
Conversation
| @@ -508,7 +509,10 @@ impl<C: Clone, T: FromJSValConvertible<Config=C>> FromJSValConvertible for Vec<T | |||
|
|
|||
| for i in 0..length { | |||
| let mut val = RootedValue::new(cx, UndefinedValue()); | |||
| assert!(JS_GetElement(cx, obj.handle(), i, val.handle_mut())); | |||
| if !JS_GetElement(cx, obj.handle(), i, val.handle_mut()) { | |||
| // On JS exception or error exit loop | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
stspyder
Feb 1, 2016
Author
Contributor
Doesn't JS_GetElement automatically report JS exceptions? I thought so because the tests with assert_throws(custom_error, fun()) were passing.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
KiChjang
Feb 1, 2016
Member
As far as I can tell, JS_GetElement is an external C function, and it doesn't use any of the throw_js_error stuff that we have written in Rust.
This comment has been minimized.
This comment has been minimized.
stspyder
Feb 1, 2016
Author
Contributor
Well, I meant the WPT tests. I thought I saw the automatic exception reporting stuff somewhere in the SpiderMonkey documentation. May be it doesn't. I'll see if I can test.
This comment has been minimized.
This comment has been minimized.
Ms2ger
Feb 2, 2016
Collaborator
I thought this couldn't fail, but I double-checked, and it can:
Object.defineProperty(Array.prototype, 1, {get: function() {throw 7}});
new Blob([,,,])…n case of JS error
|
I have updated the pull request. From JSAPI guide, it seems that JSAPI functions automatically sets the exception (thrown from JS) in the context and return false without we having to explicitly set one in the context. This exception will be cleared eventually down the stack. I have updated the code to return an Err in case of a JS exception so that the calling code can return false if the conversion fails. |
|
@bors-servo r+ |
|
|
Throw type error on converting non objects to sequence Fix for #237 <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/238) <!-- Reviewable:end -->
|
|
stspyder commentedFeb 1, 2016
Fix for #237