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

QMLProperty: Log thrown exceptions when evaluating a binding #254

Merged

Conversation

stephenmdangelo
Copy link
Member

This is more inline with the C++ QML Engine that logs the error, but
continues working.

This is more inline with the C++ QML Engine that logs the error, but
continues working.
@ChALkeR
Copy link
Member

ChALkeR commented Jun 10, 2016

It does look good to me, but just a question — what's the reason behind logging the error as-is, but stringifying the function? Note that browser console pretty-prints functions and errors, afaik. Is there something else going on there?

@stephenmdangelo
Copy link
Member Author

Ah yes, I had added the function printing as a way of trying to determine what binding was failing (as some exceptions can look very generic). This was the best way I could get it to show up in Firefox. Obviously even this can only get you so far in finding the source of the exception.

Ideally, we would mimic QML and print out the file name and line number, but I'm thinking that might be a lot of work to pass it through all of the necessary components. I could look into doing this of you think it's feasible to get line numbers from the parser.

@ChALkeR
Copy link
Member

ChALkeR commented Jun 11, 2016

This was the best way I could get it to show up in Firefox.

Ah, true. Firefox doesn't pretty-print functions in console.log().
For example, compare console.log('foo', new Error('test'), function(x) { return x * 2 }) output in Firefox and Chromium. Chromium pretty-prints the function header and body, while Firefox prints only function ().

LGTM with converting funtions to strings, then.

Ideally, we would mimic QML and print out the file name and line number, but I'm thinking that might be a lot of work to pass it through all of the necessary components.

I don't think that we should exactly mimic Qt QML in terms of error reporting to console — for some cases, it's not very nice there (e.g. for some parsing errors). Printing errors/warnings at the same time when Qt prints them is a good thing, but the exact messages could be different.

Adding line numbers to parser could be a good thing, but it will increase the parsed tree size — perhaps we could think of adding a debug flag for that. The current parsed tree structure uses simple lists, though, so adding any additional information there (moreover — optional additional information) would require significant changes in the parser. Perhaps qmlweb/qmlweb-parser#4 is a way to go, but I would postpone that.

try {
this.val = this.binding.eval(this.objectScope, this.componentScope);
} catch (e) {
console.log("QMLProperty.update binding error:", e, Function.prototype.toString.call(this.binding.eval))
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon is missing here, but we will cleanup things later.
Lint checks are not enforced yet in src/ directory.

@ChALkeR ChALkeR merged commit 021ea36 into qmlweb:master Jun 11, 2016
@ChALkeR
Copy link
Member

ChALkeR commented Jun 11, 2016

Thanks!

@ChALkeR
Copy link
Member

ChALkeR commented Jun 13, 2016

Note: it looks like that wasn't the only place where binding.eval( is executed.

@ChALkeR
Copy link
Member

ChALkeR commented Jun 13, 2016

Also take a look at 32d8a18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants