1313050 - Web console dies on logging `new Date(NaN)`

Status: VERIFIED FIXED

(DevTools :: Shared Components, defect, P1)

Description

[Simon Lindholm]

Open the Web Console, enter `new Date(NaN)` (or `new Date("something that doesn't parse")`, which comes up more often in practice). This should show "Date NaN" or "Date <invalid>" or whatever, but instead the console just silently dies and stops printing stuff, and I see the follow error in the web console:

RangeError: invalid date  reps/date-time.js:44:13

Apart from the immediate bug I feel like it would be great to have the web console be more resilient here. Seems like there are lots of opportunities for edge cases of this kind to come up, and getting an alert about an internal web console failure would be a lot better than a blank, dysfunctional console.

Comment 1

[Lin Clark [:linclark]]

Attachment: Bug1313050.patch

Thanks for filing this.

You're right, it would be better to handle these more gracefully in the web console. I'll create a separate issue for that.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8804766 [details] [diff] [review]
Bug1313050.patch

Review of attachment 8804766 [details] [diff] [review]:
-----------------------------------------------------------------

One minor comment, but looks good to me !

::: devtools/client/shared/components/reps/date-time.js
@@ +45,5 @@
>              new Date(grip.preview.timestamp).toISOString()
>            )
> +        );
> +      } catch (e) {
> +        console.error(e);

Do we want to keep this ? I think this could be confusing (for the devtoolers) to have this in the jsconsole/browser toolbox, since we output something.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Also, I think this kind of error would be a good candidate to have a [Learn More] link, even if it would require some thinking since this doesn't throw an error.

Comment 4

[Lin Clark [:linclark]]

Thanks for the review. Good point about console.error. I'll remove it. 

The Learn More link idea is interesting. It would probably end up being a larger project (figuring out the classes of things besides errors that we want to add that for), so might be a good fit for after the console rewrite is done.

Comment 5

[Lin Clark [:linclark]]

Attachment: Bug1313050.patch

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8804805 [details] [diff] [review]
Bug1313050.patch

Review of attachment 8804805 [details] [diff] [review]:
-----------------------------------------------------------------

All good!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ab0347c0213
Handle invalid dates in reps. r=nchevobbe

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ab0347c0213

Comment 9

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 52.0a1 (2016-10-26) (64-bit) on Windows 7, 64 Bit !

This bug's fix is verified with latest Nightly

Build ID    20161109030210
User Agent  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

[bugday-20161109]