<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[202657] trunk/Source/WebInspectorUI</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/202657">202657</a></dd>
<dt>Author</dt> <dd>bburg@apple.com</dd>
<dt>Date</dt> <dd>2016-06-29 16:05:55 -0700 (Wed, 29 Jun 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Web Inspector: Uncaught Exception page never shows if exception is thrown while processing a protocol event
https://bugs.webkit.org/show_bug.cgi?id=159182

Reviewed by Joseph Pecoraro.

Since we catch exceptions raised during the handling of protocol responses and events, there
is no way for these exceptions to trigger the global exception handler that shows the Uncaught
Exception Reporter sheet. We should show these in the sheet because it makes them get fixed faster.

Add a new entry point, WebInspector.reportInternalError, that takes an error or string and
a free-form map of strings to strings for storing additional information such as message data.
Pass the error and any other relevant details to this entry point, which decides whether to
show the uncaught exception reporter or quietly log the error to Inspector^2 console.

In future patches, I would like to do the following once the common errors are fixed:
 - enable reporting via Uncaught Exception Reporter for all engineering builds
 - move internal console.error call sites to use WebInspector.reportInternalError

* UserInterface/Base/Main.js: Add reportInternalError, which redirects to the uncaught
exception reporter sheet or does console.error. It also adds a console.assert that could
cause the debugger to pause if desired.

* UserInterface/Debug/UncaughtExceptionReporter.css:
(.sheet-container): Make the report scrollable now that we could potentially show a lot of text.

* UserInterface/Debug/UncaughtExceptionReporter.js:
(handleError): Also pass along the 'details' poperty.
(formattedEntry): Refactor the code so it additionally prints out the keys and values of
the 'details' property. It does not do any coercions, so callers must convert values to strings.

* UserInterface/Protocol/InspectorBackend.js:
(InspectorBackendClass.prototype._dispatchResponse): Inlined a function.
(InspectorBackendClass.prototype._dispatchResponseToCallback):
(InspectorBackendClass.prototype._dispatchEvent):
Report uncaught exceptions via WebInspector.reportInternalError.

(InspectorBackendClass.prototype._reportProtocolError): Deleted, inlined into the single use site.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceBaseMainjs">trunk/Source/WebInspectorUI/UserInterface/Base/Main.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceDebugUncaughtExceptionReportercss">trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.css</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceDebugUncaughtExceptionReporterjs">trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceProtocolInspectorBackendjs">trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (202656 => 202657)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/ChangeLog        2016-06-29 23:01:42 UTC (rev 202656)
+++ trunk/Source/WebInspectorUI/ChangeLog        2016-06-29 23:05:55 UTC (rev 202657)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2016-06-29  Brian Burg  &lt;bburg@apple.com&gt;
+
+        Web Inspector: Uncaught Exception page never shows if exception is thrown while processing a protocol event
+        https://bugs.webkit.org/show_bug.cgi?id=159182
+
+        Reviewed by Joseph Pecoraro.
+
+        Since we catch exceptions raised during the handling of protocol responses and events, there
+        is no way for these exceptions to trigger the global exception handler that shows the Uncaught
+        Exception Reporter sheet. We should show these in the sheet because it makes them get fixed faster.
+
+        Add a new entry point, WebInspector.reportInternalError, that takes an error or string and
+        a free-form map of strings to strings for storing additional information such as message data.
+        Pass the error and any other relevant details to this entry point, which decides whether to
+        show the uncaught exception reporter or quietly log the error to Inspector^2 console.
+
+        In future patches, I would like to do the following once the common errors are fixed:
+         - enable reporting via Uncaught Exception Reporter for all engineering builds
+         - move internal console.error call sites to use WebInspector.reportInternalError
+
+        * UserInterface/Base/Main.js: Add reportInternalError, which redirects to the uncaught
+        exception reporter sheet or does console.error. It also adds a console.assert that could
+        cause the debugger to pause if desired.
+
+        * UserInterface/Debug/UncaughtExceptionReporter.css:
+        (.sheet-container): Make the report scrollable now that we could potentially show a lot of text.
+
+        * UserInterface/Debug/UncaughtExceptionReporter.js:
+        (handleError): Also pass along the 'details' poperty.
+        (formattedEntry): Refactor the code so it additionally prints out the keys and values of
+        the 'details' property. It does not do any coercions, so callers must convert values to strings.
+
+        * UserInterface/Protocol/InspectorBackend.js:
+        (InspectorBackendClass.prototype._dispatchResponse): Inlined a function.
+        (InspectorBackendClass.prototype._dispatchResponseToCallback):
+        (InspectorBackendClass.prototype._dispatchEvent):
+        Report uncaught exceptions via WebInspector.reportInternalError.
+
+        (InspectorBackendClass.prototype._reportProtocolError): Deleted, inlined into the single use site.
+
</ins><span class="cx"> 2016-06-29  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Show Shadow Root type in DOM Tree
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceBaseMainjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Main.js (202656 => 202657)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Base/Main.js        2016-06-29 23:01:42 UTC (rev 202656)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Main.js        2016-06-29 23:05:55 UTC (rev 202657)
</span><span class="lines">@@ -2436,6 +2436,26 @@
</span><span class="cx">     }
</span><span class="cx"> };
</span><span class="cx"> 
</span><ins>+WebInspector.reportInternalError = function(errorOrString, details={})
+{
+    // The 'details' object includes additional information from the caller as free-form string keys and values.
+    // Each key and value will be shown in the uncaught exception reporter, console error message, or in
+    // a pre-filled bug report generated for this internal error.
+
+    let error = (errorOrString instanceof Error) ? errorOrString : new Error(errorOrString);
+    error.details = details;
+
+    // The error will be displayed in the Uncaught Exception Reporter sheet if DebugUI is enabled.
+    if (WebInspector.isDebugUIEnabled()) {
+        // This assert allows us to stop the debugger at an internal exception. It doesn't re-throw
+        // exceptions because the original exception would be lost through window.onerror.
+        // This workaround can be removed once &lt;https://webkit.org/b/158192&gt; is fixed.
+        console.assert(false, &quot;An internal exception was thrown.&quot;, error);
+        handleInternalException(error);
+    } else
+        console.error(error);
+};
+
</ins><span class="cx"> // OpenResourceDialog delegate
</span><span class="cx"> 
</span><span class="cx"> WebInspector.dialogWasDismissed = function(dialog)
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceDebugUncaughtExceptionReportercss"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.css (202656 => 202657)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.css        2016-06-29 23:01:42 UTC (rev 202656)
+++ trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.css        2016-06-29 23:05:55 UTC (rev 202657)
</span><span class="lines">@@ -31,6 +31,7 @@
</span><span class="cx">     bottom: 0;
</span><span class="cx">     z-index: var(--z-index-uncaught-exception-sheet);
</span><span class="cx">     background-color: hsl(0, 0%, 96%);
</span><ins>+    overflow: scroll;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> .uncaught-exception-sheet {
</span><span class="lines">@@ -37,7 +38,6 @@
</span><span class="cx">     padding: 50px 55px 50px 65px;
</span><span class="cx">     font-family: '-webkit-system-font';
</span><span class="cx">     font-size: 11pt;
</span><del>-    overflow-y: auto;
</del><span class="cx">     min-width: 400px;
</span><span class="cx">     color: hsl(0, 0%, 40%);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceDebugUncaughtExceptionReporterjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js (202656 => 202657)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js        2016-06-29 23:01:42 UTC (rev 202656)
+++ trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js        2016-06-29 23:05:55 UTC (rev 202657)
</span><span class="lines">@@ -57,6 +57,7 @@
</span><span class="cx">         lineNumber: error.line,
</span><span class="cx">         columnNumber: error.column,
</span><span class="cx">         stack: error.stack,
</span><ins>+        details: error.details,
</ins><span class="cx">     });
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -150,22 +151,29 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     function formattedEntry(entry) {
</span><del>-        let message = `${entry.message} (at ${entry.url}:${entry.lineNumber}:${entry.columnNumber})`;
-        if (!entry.stack)
-            return message;
-
</del><span class="cx">         const indent = &quot;    &quot;;
</span><del>-        let results = [];
-        let lines = entry.stack.split(/\n/g);
-        for (let line of lines) {
-            let atIndex = line.indexOf(&quot;@&quot;);
-            let slashIndex = Math.max(line.lastIndexOf(&quot;/&quot;), atIndex);
-            let functionName = line.substring(0, atIndex) || &quot;?&quot;;
-            let location = line.substring(slashIndex + 1, line.length);
-            results.push(`${indent}${functionName} @ ${location}`);
</del><ins>+        let lines = [`${entry.message} (at ${entry.url}:${entry.lineNumber}:${entry.columnNumber})`];
+        if (entry.stack) {
+            let stackLines = entry.stack.split(/\n/g);
+            for (let stackLine of stackLines) {
+                let atIndex = stackLine.indexOf(&quot;@&quot;);
+                let slashIndex = Math.max(stackLine.lastIndexOf(&quot;/&quot;), atIndex);
+                let functionName = stackLine.substring(0, atIndex) || &quot;?&quot;;
+                let location = stackLine.substring(slashIndex + 1, stackLine.length);
+                lines.push(`${indent}${functionName} @ ${location}`);
+            }
</ins><span class="cx">         }
</span><span class="cx"> 
</span><del>-        return message + &quot;\n&quot; + results.join(&quot;\n&quot;);
</del><ins>+        if (entry.details) {
+            lines.push(&quot;&quot;);
+            lines.push(&quot;Additional Details:&quot;)
+            for (let key in entry.details) {
+                let value = entry.details[key];
+                lines.push(`${indent}${key} --&gt; ${value}`);
+            }
+        }
+
+        return lines.join(&quot;\n&quot;);
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     let inspectedPageURL = null;
</span><span class="lines">@@ -232,6 +240,6 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> window.addEventListener(&quot;error&quot;, handleUncaughtException);
</span><del>-window.handlePromiseException = handleError;
</del><ins>+window.handlePromiseException = window.handleInternalException = handleError;
</ins><span class="cx"> 
</span><span class="cx"> })();
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceProtocolInspectorBackendjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js (202656 => 202657)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js        2016-06-29 23:01:42 UTC (rev 202656)
+++ trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js        2016-06-29 23:05:55 UTC (rev 202657)
</span><span class="lines">@@ -255,7 +255,7 @@
</span><span class="cx"> 
</span><span class="cx">         if (messageObject[&quot;error&quot;]) {
</span><span class="cx">             if (messageObject[&quot;error&quot;].code !== -32000)
</span><del>-                this._reportProtocolError(messageObject);
</del><ins>+                console.error(&quot;Request with id = &quot; + messageObject[&quot;id&quot;] + &quot; failed. &quot; + JSON.stringify(messageObject[&quot;error&quot;]));
</ins><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         let sequenceId = messageObject[&quot;id&quot;];
</span><span class="lines">@@ -298,7 +298,10 @@
</span><span class="cx">         try {
</span><span class="cx">             callback.apply(null, callbackArguments);
</span><span class="cx">         } catch (e) {
</span><del>-            console.error(&quot;Uncaught exception in inspector page while dispatching callback for command &quot; + command.qualifiedName, e);
</del><ins>+            WebInspector.reportInternalError(e, {
+                &quot;cause&quot;: `An uncaught exception was thrown while dispatching response callback for command ${command.qualifiedName}.`,
+                &quot;protocol-message&quot;: JSON.stringify(messageObject),
+            });
</ins><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="lines">@@ -343,9 +346,13 @@
</span><span class="cx">         try {
</span><span class="cx">             agent.dispatchEvent(eventName, eventArguments);
</span><span class="cx">         } catch (e) {
</span><del>-            console.error(&quot;Uncaught exception in inspector page while handling event &quot; + qualifiedName, e);
</del><span class="cx">             for (let tracer of this.activeTracers)
</span><span class="cx">                 tracer.logFrontendException(messageObject, e);
</span><ins>+
+            WebInspector.reportInternalError(e, {
+                &quot;cause&quot;: `An uncaught exception was thrown while handling event: ${qualifiedName}`,
+                &quot;protocol-message&quot;: JSON.stringify(messageObject),
+            });
</ins><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         let processingDuration = (timestamp() - processingStartTimestamp).toFixed(3);
</span><span class="lines">@@ -353,11 +360,6 @@
</span><span class="cx">             tracer.logDidHandleEvent(messageObject, {dispatch: processingDuration});
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    _reportProtocolError(messageObject)
-    {
-        console.error(&quot;Request with id = &quot; + messageObject[&quot;id&quot;] + &quot; failed. &quot; + JSON.stringify(messageObject[&quot;error&quot;]));
-    }
-
</del><span class="cx">     _flushPendingScripts()
</span><span class="cx">     {
</span><span class="cx">         console.assert(this._pendingResponses.size === 0);
</span></span></pre>
</div>
</div>

</body>
</html>