Bug 159182 - Web Inspector: Uncaught Exception page never shows if exception is thrown while processing a protocol event
Summary: Web Inspector: Uncaught Exception page never shows if exception is thrown whi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Brian Burg
URL:
Keywords: DoNotImportToRadar
Depends on:
Blocks:
 
Reported: 2016-06-27 17:12 PDT by Brian Burg
Modified: 2016-06-29 16:06 PDT (History)
6 users (show)

See Also:


Attachments
Proposed Fix (10.54 KB, patch)
2016-06-28 17:48 PDT, Brian Burg
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2016-06-27 17:12:08 PDT
We catch, log, and drop the assertion on the floor. We should probably only do that if !DebugUIEnabled, so that we can easily file and fix these exceptions.
Comment 1 Brian Burg 2016-06-27 17:12:16 PDT
exception*
Comment 2 Brian Burg 2016-06-28 17:48:01 PDT
Created attachment 282307 [details]
Proposed Fix
Comment 3 Joseph Pecoraro 2016-06-28 18:20:45 PDT
Comment on attachment 282307 [details]
Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Base/Main.js:2456
> +    } else if (optionalMessage)
> +        console.error(optionalMessage, error);

This `optionalMessage` stuff seems stale? Right now it looks like a ReferenceError.

> Source/WebInspectorUI/UserInterface/Base/Main.js:2459
> +}

Style: ;

> Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js:173
> +            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}`);
> +                }
> +            }

Seems this can be outside of the "if (entry.stack)" block.

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:305
> +            WebInspector.reportInternalError(e, details);

Style: Inlining the details would look nicer!

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:357
> +            WebInspector.reportInternalError(e, details);

Style: Inlining the details would look nicer!
Comment 4 Brian Burg 2016-06-29 16:06:19 PDT
Committed r202657: <http://trac.webkit.org/changeset/202657>