Viewing some old Twitter logs breaks log viewer

RESOLVED FIXED in Thunderbird 17.0

Status

Thunderbird
Instant Messaging
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: florian)

Tracking

15 Branch
Thunderbird 17.0
x86
Windows 7

Thunderbird Tracking Flags

(thunderbird15 fixed, thunderbird16 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
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
(Reporter)

Comment 2

5 years ago
This seems to affect the current beta.
Version: Trunk → 15
(Reporter)

Comment 3

5 years ago
(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.)
(Assignee)

Comment 4

5 years ago
Created attachment 650187 [details] [diff] [review]
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).
Assignee: nobody → florian
Attachment #650187 - Flags: review?(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!
Attachment #650187 - Flags: review?(clokep) → review+
(Assignee)

Comment 6

5 years ago
(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.
(Assignee)

Comment 7

5 years ago
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.
Attachment #650187 - Flags: approval-comm-beta?
Attachment #650187 - Flags: approval-comm-aurora?
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.
Attachment #650187 - Flags: approval-comm-beta?
Attachment #650187 - Flags: approval-comm-beta+
Attachment #650187 - Flags: approval-comm-aurora?
Attachment #650187 - Flags: approval-comm-aurora+
(Reporter)

Comment 9

5 years ago
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
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-thunderbird15: --- → fixed
status-thunderbird16: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
(Assignee)

Comment 10

5 years ago
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
You need to log in before you can comment on or make changes to this bug.