Bug 56100 - Web Inspector: introduce error argument for backend callbacks
Summary: Web Inspector: introduce error argument for backend callbacks
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: Ilya Tikhonovsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-10 07:28 PST by Ilya Tikhonovsky
Modified: 2011-03-11 11:55 PST (History)
13 users (show)

See Also:


Attachments
[patch] initial version (48.25 KB, patch)
2011-03-10 07:32 PST, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
[patch] second version (67.90 KB, patch)
2011-03-11 04:43 PST, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2011-03-10 07:28:39 PST
%subj%
Comment 1 Ilya Tikhonovsky 2011-03-10 07:32:15 PST
Created attachment 85323 [details]
[patch] initial version
Comment 2 Patrick Mueller 2011-03-10 08:27:13 PST
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 ...
Comment 3 Ilya Tikhonovsky 2011-03-10 12:47:53 PST
(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.
Comment 4 Ilya Tikhonovsky 2011-03-10 12:59:43 PST
(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)
Comment 5 Ilya Tikhonovsky 2011-03-11 04:43:29 PST
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.
Comment 6 Yury Semikhatsky 2011-03-11 05:24:49 PST
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?
Comment 7 WebKit Review Bot 2011-03-11 06:27:27 PST
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
Comment 8 Ilya Tikhonovsky 2011-03-11 11:55:33 PST
was landed as r80845