Bug 59000 – Web Inspector: [protocol] Evaluate should expose thrown exception value

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Peter Rybin]

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".

[Yury Semikhatsky]

Created attachment 91511 [details]
Patch

[Pavel Feldman]

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?

[Build Bot]

Attachment 91511 [details] did not build on win:
Build output: http://queues.webkit.org/results/8520423

[Yury Semikhatsky]

Created attachment 91668 [details]
Patch for landing

All comments addressed.

[Yury Semikhatsky]

Committed r85319: <http://trac.webkit.org/changeset/85319>

[Peter Rybin]

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.