[GTK] Provide details on javascript exception

[David Keijser]

Running `webkit_web_view_run_javascript` with a script that fails the error object returned contains no information on what went wrong but only the message "An exception was raised in JavaScript". Having access to at least the javascript error message would help debugging a lot.

[David Keijser]

Created attachment 310896 [details]
Patch

[Build Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Comment on attachment 310896 [details]
Patch

Wow nice, thanks! This looks perfect to me. I'll let this sit a day to give someone else a chance to review it too; please complain here if nobody else responds soon.

Also, you probably want to set the cq? flag to request commit.

[Carlos Garcia Campos]

Comment on attachment 310896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310896&action=review

Thanks for the patch.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:3139
> -            _("An exception was raised in JavaScript"));
> +            _("An exception was raised in JavaScript: %s"), exceptionDetails.message.utf8().data());

Now that we have the error details, I think the prefix "An exception was raised in JavaScript" is kind of redundant, we know that from the error code WEBKIT_JAVASCRIPT_ERROR_SCRIPT_FAILED. I think we can use all the details to build a complete error message, consistent with messages shown by the inspector or stdout when setting enable-write-console-messages-to-stdout is enabled. If we have a url, then we add url with line and column, and then error message, if we don't have a url we just show the error message. You can see how the message is built in Source/JavaScriptCore/runtime/ConsoleClient.cpp

[David Keijser]

Created attachment 310990 [details]
Patch

[David Keijser]

Thanks for the review, I have updated the patch.

However I've noted that the sourceURL is always "undefined", line numbers works though. I'm not sure how to get that field properly populated.

[Carlos Garcia Campos]

(In reply to David Keijser from comment #6)
> Thanks for the review, I have updated the patch.
> 
> However I've noted that the sourceURL is always "undefined", line numbers
> works though. I'm not sure how to get that field properly populated.

You mean it's empty or that it's the string "undefined"? It sounds like a bug, then.

[Carlos Garcia Campos]

Comment on attachment 310990 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310990&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:3152
> +        builder.append(exceptionDetails.message.utf8().data());

Do not convert to utf8 here

builder.append(exceptionDetails.message);

[David Keijser]

it's always set and always the string "undefined", sry for the confusion :)

[David Keijser]

Created attachment 311106 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 311106 [details]
Patch

We need to investigate why source URL is "undefined", but that's a different bug in any case. Could you file a new bug report or that, please?

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3) 
> Also, you probably want to set the cq? flag to request commit.

[WebKit Commit Bot]

Comment on attachment 311106 [details]
Patch

Clearing flags on attachment: 311106

Committed r217389: <http://trac.webkit.org/changeset/217389>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Keijser]

I've created the issue https://bugs.webkit.org/show_bug.cgi?id=172587