777873 - Viewing some old Twitter logs breaks log viewer

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

STR:

In the Chat tab, choose a previous conversation from several weeks ago

What happens?

The log is not displayed. What's more, viewing *any* logs at this point is broken.

What's expected?

The log viewer should work.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

This appears in the Error Console:

Timestamp: 26/07/2012 3:58:01 PM
Error: TypeError: aConv is null
Source File: chrome://chat/content/convbrowser.xml
Line: 713

Comment 2

[Mike Conley (:mconley) (:⚙️)]

This seems to affect the current beta.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(I'm unsure how critical an error this is - perhaps my old logs are corrupt somehow...they go all the way back to April 1st. Regardless, we've clearly got a hole in our error handling here.)

Comment 4

[Florian Quèze [:florian]]

Attachment: Patch

This patch handles 3 error cases:
- log files that aren't in the JSON format (I think Mike may have tested a very old try server build of IM in Tb that still wrote plain text logs. The error in the console in that case is exactly what Mike reported)
- random files in the log folder that don't have a name matching the expected format (these files are now just ignored, instead of breaking the list of logs)
- random junk inside a log file with a correct name and a .json extension. (The log file will be ignored if the first line is not valid JSON. If the junk starts later, only the lines that aren't valid JSON will be ignored).

Comment 5

[Patrick Cloke [:clokep]]

Comment on attachment 650187 [details] [diff] [review]
Patch

(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> This patch handles 3 error cases:
> - log files that aren't in the JSON format (I think Mike may have tested a
> very old try server build of IM in Tb that still wrote plain text logs. The
> error in the console in that case is exactly what Mike reported)

Shouldn't these go into the text log reader we have then or am I misunderstanding that?

The changes here look fine though!

Comment 6

[Florian Quèze [:florian]]

(In reply to Patrick Cloke [:clokep] from comment #5)

> > - log files that aren't in the JSON format (I think Mike may have tested a
> > very old try server build of IM in Tb that still wrote plain text logs. The
> > error in the console in that case is exactly what Mike reported)
> 
> Shouldn't these go into the text log reader we have then or am I
> misunderstanding that?

Thunderbird never logged in the plain text format (except for a few very early testing builds) so it doesn't have the text log reader that Instantbird has.

Comment 7

[Florian Quèze [:florian]]

Comment on attachment 650187 [details] [diff] [review]
Patch

[Approval Request Comment]
User impact if declined: parts of the chat UI will be broken if the user has random junk in a log folder.

Risk to taking this patch: low.

Should we take this for Tb15? I really don't know. The risk is low (the patch mostly adds null checks and try/catch in a few places), but the benefit seems only marginal (ie, why would users have random invalid log files in their log folders?).

I'm requesting approval mostly so that someone actually make a decision here.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 650187 [details] [diff] [review]
Patch

Although there's probably not too many people with corrupted logs, I think keeping working if we get into that situation is probably a good idea. We don't know yet the full extent of what we'll hit, so lets take it and have less risk there.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

comm-central: https://hg.mozilla.org/comm-central/rev/4d1d1564495a
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/f1d65ccc71a9
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/728b2c0854a4

Comment 10

[Florian Quèze [:florian]]

The chat/ part landed for Instantbird as http://hg.instantbird.org/instantbird/rev/8ec7e45f71fd and the UI changes were dealt with in https://bugzilla.instantbird.org/show_bug.cgi?id=1448