Bug 56100 – Web Inspector: introduce error argument for backend callbacks

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

[Ilya Tikhonovsky]

%subj%

[Ilya Tikhonovsky]

Created attachment 85323 [details]
[patch] initial version

[Patrick Mueller]

So all callbacks now have a free "error" parameter as the first parameter.

Not clear what's expected to be serialized for "error", just given the patch.  Most (all?) of the checks in the patch for error seem to be checking it for truth-y ness and expecting a valid string value (via toString() maybe).  Is it as simple as "error" is a String, or can it be a richer object that can coerce itself to a nice string?  Wasn't quite sure where to look in the code for the answer ...

[Ilya Tikhonovsky]

(In reply to comment #2)

A month ago only InspectorBackendDispatcher was able to produce error messages in order of validation incoming messages.

About two weeks ago the error argument was introduced for the all Agents' methods on the backend side. And now the agents' functions can return a custom error text as the result of an operation. 

But there is a problem here. If the error string is not empty it will be added to the list of errors and transfered to the frontend the same way as the message validation errors. 
At the frontend side all such errors will be printed into Inspector's inspector console and the callback will be never called.

this patch is a way to solve this problem.

It is not the final version of patch. The custom error text will be transfered as another field of the protocol messages just for separating protocol errors and business errors or marked as a 'business' error some other way.

[Ilya Tikhonovsky]

(In reply to comment #3)
> (In reply to comment #2)

> But there is a problem here. If the error string is not empty it will be added to the list of errors and transfered to the frontend the same way as the message validation errors. 
> At the frontend side all such errors will be printed into Inspector's inspector console and the callback will be never called.


Two problems:
1) we don't call the callback in case of an error in the message; (in the patch)
2) It is not possible to distinguish custom error and protocol error at frontend side. (will do that in the next version of patch)

[Ilya Tikhonovsky]

Created attachment 85460 [details]
[patch] second version

1) response message field 'errors' was renamed to 'protocolErrors';
2) new field 'error' was introduced for the response messages. It is used for transfer custom error's text from the backend agents' functions to frontend callbacks; 
3) 'seq' field in the request messages was renamed to 'id';
4) 'seq' field in the response messages was renamed to 'requestId';

there is no tests for custom-error functionality.
I'll do that in the next patch.

[Yury Semikhatsky]

Comment on attachment 85460 [details]
[patch] second version

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

> LayoutTests/inspector/elements/dom-agent-query-selector.html:44
>          if (!(nodeIds instanceof Array))

Should this code be executed in case of error?

[WebKit Review Bot]

http://trac.webkit.org/changeset/80845 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
svg/custom/clip-path-referencing-use2.svg
tables/mozilla/bugs/bug22246-2a.html

[Ilya Tikhonovsky]

was landed as r80845