Bug 49488 - Only add source specific information to exceptions in Interpreter::throwException
Summary: Only add source specific information to exceptions in Interpreter::throwExcep...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-12 20:24 PST by Gavin Barraclough
Modified: 2010-11-15 17:31 PST (History)
1 user (show)

See Also:


Attachments
Code changes (41.66 KB, patch)
2010-11-13 02:20 PST, Gavin Barraclough
ggaren: review+
Details | Formatted Diff | Diff
Test changes (156.11 KB, patch)
2010-11-13 02:20 PST, Gavin Barraclough
no flags Details | Formatted Diff | Diff
Test changes II (file upload failed) (140.02 KB, patch)
2010-11-13 02:24 PST, Gavin Barraclough
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2010-11-12 20:24:30 PST
Three types of source location information are added to errors.

(1) Divot information.
This was added with the intention of using it to provide better source highlighting in the inspector.  We may still want to do so, but we probably should not be exposing these values in a manner visible to user scripts – only through an internal C++ interface.  The code adding divot properties to objects has been removed.

(2) Line number information.
Line number information is presently sometimes added at the point the exception is created, and sometimes added at the point the exception passes through throwException.  Change this so that throwException has the sole responsibility for adding line number and source file information.

(3) Source snippets in the message of certain type errors (e.g. 'doc' in `Result of expression 'doc' [undefined] is not an object.`).
These messages are currently created at the point the exceptions is raised.  Instead reformat the message such that the source snippet is located at the end (`Result of expression 'b1' [undefined] is not an object.` -> `'undefined' is not an object (evaluating 'b1.property')`), and append these to the message at the in throw Exception.  This presents a number of advantages:

i) we no longer need to have source location information to create these TypeErrors.
ii) we can chose to append source location information in other error messages, including those where passing source location to the point of construction would be inconvenient.
iii) we can chose in future to omit to append source location information when running in a non-debug mode.

This also cleans up some error output, e.g. removing double brackets ('[[]]') around objects in output, removing double periods (..) at end of lines, and adding slightly more context to some errors.
Comment 1 Gavin Barraclough 2010-11-13 02:20:22 PST
Created attachment 73813 [details]
Code changes
Comment 2 Gavin Barraclough 2010-11-13 02:20:58 PST
Created attachment 73814 [details]
Test changes
Comment 3 WebKit Review Bot 2010-11-13 02:22:53 PST
Attachment 73814 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Gavin Barraclough 2010-11-13 02:24:25 PST
Created attachment 73815 [details]
Test changes II (file upload failed)
Comment 5 Geoffrey Garen 2010-11-15 11:38:06 PST
Comment on attachment 73813 [details]
Code changes

r=me
Comment 6 Geoffrey Garen 2010-11-15 11:38:15 PST
Comment on attachment 73815 [details]
Test changes II (file upload failed)

r=me
Comment 7 Gavin Barraclough 2010-11-15 17:31:18 PST
Committed revision 72050.