Changeset 202657 in webkit

Timestamp:

Jun 29, 2016 4:05:55 PM (8 years ago)

Author:

BJ Burg

Message:

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.

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• UserInterface/Base/Main.js (modified) (1 diff)
• UserInterface/Debug/UncaughtExceptionReporter.css (modified) (2 diffs)
• UserInterface/Debug/UncaughtExceptionReporter.js (modified) (3 diffs)
• UserInterface/Protocol/InspectorBackend.js (modified) (4 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2016-06-29  Brian Burg  <bburg@apple.com>
+
+        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.
+
 2016-06-29  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Base/Main.js

@@ -2437 +2437 @@
 };
 
+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 <https://webkit.org/b/158192> is fixed.
+        console.assert(false, "An internal exception was thrown.", error);
+        handleInternalException(error);
+    } else
+        console.error(error);
+};
+
 // OpenResourceDialog delegate
 

trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.css

@@ -32 +32 @@
     z-index: var(--z-index-uncaught-exception-sheet);
     background-color: hsl(0, 0%, 96%);
+    overflow: scroll;
 }
 
@@ -38 +39 @@
     font-family: '-webkit-system-font';
     font-size: 11pt;
-    overflow-y: auto;
     min-width: 400px;
     color: hsl(0, 0%, 40%);

trunk/Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js

@@ -58 +58 @@
         columnNumber: error.column,
         stack: error.stack,
+        details: error.details,
     });
 }
@@ -151 +152 @@
 
     function formattedEntry(entry) {
-        let message = `${entry.message} (at ${entry.url}:${entry.lineNumber}:${entry.columnNumber})`;
-        if (!entry.stack)
-            return message;
-
         const indent = "    ";
-        let results = [];
-        let lines = entry.stack.split(/\n/g);
-        for (let line of lines) {
-            let atIndex = line.indexOf("@");
-            let slashIndex = Math.max(line.lastIndexOf("/"), atIndex);
-            let functionName = line.substring(0, atIndex) || "?";
-            let location = line.substring(slashIndex + 1, line.length);
-            results.push(`${indent}${functionName} @ ${location}`);
+        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("@");
+                let slashIndex = Math.max(stackLine.lastIndexOf("/"), atIndex);
+                let functionName = stackLine.substring(0, atIndex) || "?";
+                let location = stackLine.substring(slashIndex + 1, stackLine.length);
+                lines.push(`${indent}${functionName} @ ${location}`);
+            }
         }
 
-        return message + "\n" + results.join("\n");
+        if (entry.details) {
+            lines.push("");
+            lines.push("Additional Details:")
+            for (let key in entry.details) {
+                let value = entry.details[key];
+                lines.push(`${indent}${key} --> ${value}`);
+            }
+        }
+
+        return lines.join("\n");
     }
 
@@ -233 +241 @@
 
 window.addEventListener("error", handleUncaughtException);
-window.handlePromiseException = handleError;
+window.handlePromiseException = window.handleInternalException = handleError;
 
 })();

trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js

@@ -256 +256 @@
         if (messageObject["error"]) {
             if (messageObject["error"].code !== -32000)
-                this._reportProtocolError(messageObject);
+                console.error("Request with id = " + messageObject["id"] + " failed. " + JSON.stringify(messageObject["error"]));
         }
 
@@ -299 +299 @@
             callback.apply(null, callbackArguments);
         } catch (e) {
-            console.error("Uncaught exception in inspector page while dispatching callback for command " + command.qualifiedName, e);
+            WebInspector.reportInternalError(e, {
+                "cause": `An uncaught exception was thrown while dispatching response callback for command ${command.qualifiedName}.`,
+                "protocol-message": JSON.stringify(messageObject),
+            });
         }
     }
@@ -344 +347 @@
             agent.dispatchEvent(eventName, eventArguments);
         } catch (e) {
-            console.error("Uncaught exception in inspector page while handling event " + qualifiedName, e);
             for (let tracer of this.activeTracers)
                 tracer.logFrontendException(messageObject, e);
+
+            WebInspector.reportInternalError(e, {
+                "cause": `An uncaught exception was thrown while handling event: ${qualifiedName}`,
+                "protocol-message": JSON.stringify(messageObject),
+            });
         }
 
@@ -352 +359 @@
         for (let tracer of this.activeTracers)
             tracer.logDidHandleEvent(messageObject, {dispatch: processingDuration});
-    }
-
-    _reportProtocolError(messageObject)
-    {
-        console.error("Request with id = " + messageObject["id"] + " failed. " + JSON.stringify(messageObject["error"]));
     }