Bug 59000 - Web Inspector: [protocol] Evaluate should expose thrown exception value
Summary: Web Inspector: [protocol] Evaluate should expose thrown exception value
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on: 59611
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-20 09:15 PDT by Peter Rybin
Modified: 2019-05-02 16:18 PDT (History)
12 users (show)

See Also:


Attachments
Patch (42.48 KB, patch)
2011-04-28 10:16 PDT, Yury Semikhatsky
pfeldman: review+
Details | Formatted Diff | Diff
Patch for landing (44.73 KB, patch)
2011-04-29 05:26 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Rybin 2011-04-20 09:15:19 PDT
In the protocol "evaluateOnCallFrame" command returns result value.

However if expression throw exception, you only get string representation of exception (or get the process crashed if value is 'undefined').

The protocol should return value that has been thrown from expression. E.g. it may keep "result" property in result and add additional property "wasThrown".
Comment 1 Yury Semikhatsky 2011-04-28 10:16:00 PDT
Created attachment 91511 [details]
Patch
Comment 2 Pavel Feldman 2011-04-28 10:26:47 PDT
Comment on attachment 91511 [details]
Patch

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

> LayoutTests/inspector/protocol/runtime-agent-expected.txt:26
> +        wasThrown : false

Could you make this optional?

> Source/WebCore/inspector/InjectedScript.cpp:214
> +        *errorString = "Internal error";

Could you add more details to this message?

> Source/WebCore/inspector/InjectedScript.cpp:223
> +        *errorString = "Internal error";

ditto

> Source/WebCore/inspector/Inspector.json:214
> +                    { "name": "valueWasThrown", "type": "boolean", "optional": true, "description": "True if exception was thrown on attempt to get the property value, in that case the value propery will contain thrown value." },

wasThrown

> Source/WebCore/inspector/front-end/ConsoleView.js:363
> +            if (!result || wasThrown)

is this check needed?
Comment 3 Build Bot 2011-04-28 10:56:54 PDT
Attachment 91511 [details] did not build on win:
Build output: http://queues.webkit.org/results/8520423
Comment 4 Yury Semikhatsky 2011-04-29 05:26:01 PDT
Created attachment 91668 [details]
Patch for landing

All comments addressed.
Comment 5 Yury Semikhatsky 2011-04-29 05:37:49 PDT
Committed r85319: <http://trac.webkit.org/changeset/85319>
Comment 6 Peter Rybin 2011-05-12 16:20:09 PDT
Could you please consider refinement here:
compilation failure and thrown exception should be reported differently.

Compilation failure could be returned as exception (annotated for example as thrown="compile") or as rpc failure.

I makes sense to display these problems differently.