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

Sort out throwing errors from DOM bindings #347

Closed
jdm opened this issue Apr 8, 2013 · 11 comments
Closed

Sort out throwing errors from DOM bindings #347

jdm opened this issue Apr 8, 2013 · 11 comments
Labels
A-content/bindings The DOM bindings C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor. I-wrong An incorrect behaviour is observed.

Comments

@jdm
Copy link
Member

jdm commented Apr 8, 2013

There are lots of XXXs and commented out calls to js_GetErrorMessage in the generated code. We need to sort out what to do, with the end goal of throwing the right kind of error according the the relevant spec.

@jdm
Copy link
Member Author

jdm commented Jun 26, 2013

The easiest path forward for now is to add some limited wrappers in rust-mozjs that call JS_ReportError with well-defined sets of parameters, according to our needs.

@jdm
Copy link
Member Author

jdm commented Aug 31, 2013

@bfrohs This would be a really useful piece of work you could do. The generated bindings code is full of places where we just return 0 with a commented out call to JS_ReportError.

@0b10011
Copy link
Contributor

0b10011 commented Aug 31, 2013

Sounds good. After I get my two current PRs finished up, I'll look into it more. Thanks for the suggestion!

@jdm
Copy link
Member Author

jdm commented Nov 5, 2013

As of rust-lang/rust#2057 we can now write bindings for variadic functions! \o/

@brunoabinader
Copy link
Contributor

I wonder if #1542 is a duplicate of this issue - DOMException skeleton is now available.

@jdm
Copy link
Member Author

jdm commented Feb 10, 2014

No, I'm pretty certain that DOMException interacts with error event handlers on the content side, whereas this issue revolves around fixing the places in generated code where we currently just return false without calling anything like JS_ReportError.

@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 10, 2014

There's still issues with DOMException too; we throw a plain Error right now (in throw_method_failed_with_details).

@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 9, 2014

Current status: we have throw_type_error working correctly; we just need someone to replace the XXXjdm comments in CodegenRust.py by throw_type_error calls (with strings from src/components/script/dom/bindings/codegen/Errors.msg).

@plaublin
Copy link

I am currently working on this problem.

@plaublin
Copy link

plaublin commented Aug 9, 2014

I was going to do a pull request but found out that the code changed a little bit since last month.
In my branch I call throw_type_error, but in the main branch there are some comments to ThrowErrorMessage (diff: https://github.com/schaars/servo/compare/dom_throw_error). So I was wondering if I should modify the calls to throw_type_error by calls to ThrowErrorMessage or not?

@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 9, 2014

No, those changes look good to me. (Please do rebase and squash your branch before creating the PR.)

plaublin pushed a commit to plaublin/servo that referenced this issue Aug 9, 2014
plaublin pushed a commit to plaublin/servo that referenced this issue Aug 9, 2014
@Ms2ger Ms2ger closed this as completed in cf734b0 Sep 4, 2014
jdm added a commit that referenced this issue Sep 4, 2014
Throw TypeErrors instead of uncatcheable exceptions in CodegenRust.py (fixes #347, #3065).
ChrisParis pushed a commit to ChrisParis/servo that referenced this issue Sep 7, 2014
Use assert_equals in document.body-setter-01.html.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/bindings The DOM bindings C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor. I-wrong An incorrect behaviour is observed.
Projects
None yet
Development

No branches or pull requests

5 participants