1361270 - Avoid exception when searching in the Console panel

Status: VERIFIED FIXED

(DevTools :: Console, defect, P1)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

This is a follow up for bug 1307879

STR:
1) Open the Toolbox and select the Console panel
2) Make sure there are some HTTP request logs 
3) Try to search using free-text
4) Check out the following error in the Browser Console

TypeError: message.messageText is undefined
resource://devtools/client/webconsole/new-console-output/selectors/messages.js, line: 132


This happens because:

`message.messageText !== null` is true in case `typeof message.messageText == "undefined"`

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1361270 - Avoid exception when searching in the Console panel;

Review commit: https://reviewboard.mozilla.org/r/135402/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135402/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8863612 [details]
Bug 1361270 - Avoid exception when searching in the Console panel;

https://reviewboard.mozilla.org/r/135402/#review138342

Seems good to me.
Could you add a test case where we filter a message with `messageText === undefined` (e.g. a network message) to make sure we don't run into this later ? Thanks

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1361270 - Search also within net logs;

Review commit: https://reviewboard.mozilla.org/r/135480/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135480/

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1361270 - Update test;

Review commit: https://reviewboard.mozilla.org/r/135482/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135482/

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> Comment on attachment 8863612 [details]
> Bug 1361270 - Avoid exception when searching in the Console panel;
> 
> https://reviewboard.mozilla.org/r/135402/#review138342
> 
> Seems good to me.
> Could you add a test case where we filter a message with `messageText ===
> undefined` (e.g. a network message) to make sure we don't run into this
> later ? Thanks
Attached

There is also a patch that fixes search in Net events + code clean up

Honza

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8863702 [details]
Bug 1361270 - Update test;

https://reviewboard.mozilla.org/r/135482/#review138470

::: devtools/client/webconsole/package.json:9
(Diff revision 1)
>    "engines": {
>      "node": ">=6.9.0"
>    },
>    "scripts": {
>      "start": "node bin/dev-server",
> -    "test": "cross-env NODE_ENV=test NODE_PATH=../../../ mocha new-console-output/test/**/*.test.js --compilers js:babel-register -r jsdom-global/register -r ./new-console-output/test/require-helper.js"
> +    "test": "cross-env NODE_ENV=test NODE_PATH=../../../ mocha new-console-output/test/**/search.test.js --compilers js:babel-register -r jsdom-global/register -r ./new-console-output/test/require-helper.js"

I think this shouldn't be commited

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8863701 [details]
Bug 1361270 - Search also within net logs;

https://reviewboard.mozilla.org/r/135480/#review138472

Looks better, thanks for cleaning this up

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8863702 [details]
Bug 1361270 - Update test;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135482/diff/1-2/

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #6)
> > -    "test": "cross-env NODE_ENV=test NODE_PATH=../../../ mocha new-console-output/test/**/*.test.js --compilers js:babel-register -r jsdom-global/register -r ./new-console-output/test/require-helper.js"
> > +    "test": "cross-env NODE_ENV=test NODE_PATH=../../../ mocha new-console-output/test/**/search.test.js --compilers js:babel-register -r jsdom-global/register -r ./new-console-output/test/require-helper.js"
> 
> I think this shouldn't be commited
Fixed, thanks!

Honza

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8863702 [details]
Bug 1361270 - Update test;

https://reviewboard.mozilla.org/r/135482/#review138476

Thanks, r+

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8863701 [details]
Bug 1361270 - Search also within net logs;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135480/diff/1-2/

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8863702 [details]
Bug 1361270 - Update test;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135482/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70aea1eae002
Avoid exception when searching in the Console panel; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/be7d42a610ff
Search also within net logs; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/8726b2d0277e
Update test; r=nchevobbe

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/70aea1eae002
https://hg.mozilla.org/mozilla-central/rev/be7d42a610ff
https://hg.mozilla.org/mozilla-central/rev/8726b2d0277e

Comment 15

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Confirming that 55.0a1 (2017-05-17) build is verified fixed on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6.