Closed
Bug 955526
Opened 10 years ago
Closed 10 years ago
Debug logs are binary only
Categories
(Chat Core :: Yahoo! Messenger, defect)
Chat Core
Yahoo! Messenger
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: qheaden)
References
Details
Attachments
(1 file, 2 obsolete files)
13.20 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2089 at 2013-08-02 11:42:00 UTC *** It might be useful to add some human-readable debug logging: e.g. when a message is sent, include a log entry with the message in plaintext rather than binary. You may WONTFIX this, but I thought it was worth discussing.
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 2089 at 2013-09-03 16:41:25 UTC *** Debug logs also include NO incoming data at all. (Apart from the logging the socket does.) This makes them fairly useless.
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 2089 as attmnt 2822 at 2013-09-04 20:49:00 UTC *** This patch adds human readable logging for incoming and outgoing packets. The log messages contain info about the packet's service type, status number, session ID, and the key/value pair data that is contained within it.
Attachment #8354592 -
Flags: review?(clokep)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Comment 3•10 years ago
|
||
Comment on attachment 8354592 [details] [diff] [review] Patch 1 *** Original change on bio 2089 attmnt 2822 at 2013-09-05 02:28:06 UTC *** >diff --git a/chat/protocols/yahoo/yahoo-session.jsm b/chat/protocols/yahoo/yahoo-session.jsm >@@ -167,25 +167,25 @@ YahooSession.prototype = { >- this.sendBinaryData(packet.toArrayBuffer()); >+ this.sendBinaryData(packet.toArrayBuffer(), packet.toString()); Code duplication alert! Let's add a sendPacket method that calls sendBinaryData(packet.toArrayBuffer(), packet.toString()) >@@ -756,16 +757,29 @@ YahooPacket.prototype = { >+ toString: function() { >+ let s = ""; Just set s = to the first line to start with. >+ // First, add packet header information. >+ s += "Service: " + this.service.toString(16) + "\n"; >+ s += "Status: " + this.status.toString(16) + "\n"; >+ s += "Session ID: " + this.sessionId.toString(16) + "\n\n"; These should probably have a 0x in front of them if they're hex. >+ // Now we add the packet data. >+ s += "Packet Key-Value Data:\n"; >+ for each (let pair in this.keyValuePairs) >+ s += pair.key + ":\t" + pair.value + "\n"; What if there are no key-value pairs? Can that happen?
Attachment #8354592 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 2089 as attmnt 2827 at 2013-09-05 16:41:00 UTC *** This patch addresses the previous review points.
Attachment #8354597 -
Flags: review?(clokep)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8354592 [details] [diff] [review] Patch 1 *** Original change on bio 2089 attmnt 2822 at 2013-09-05 16:41:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354592 -
Attachment is obsolete: true
Comment 6•10 years ago
|
||
Comment on attachment 8354597 [details] [diff] [review] Patch 2 *** Original change on bio 2089 attmnt 2827 at 2013-09-05 16:51:15 UTC *** My only comment would be that it's kind of weird that there will be two hanging \n if there are no key-value pairs. I don't have a strong feeling against this though. Let's give this a try, if we think we can improve it later we'll do that.
Attachment #8354597 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 2089 as attmnt 2835 at 2013-09-06 05:48:00 UTC *** Made some small fixes as per the conversation here: http://log.bezut.info/instantbird/130906/#m6. I also addressed the feedback given in comment 5. Since Florian brought up the suggestion in the IRC, I've given him the review this time.
Attachment #8354605 -
Flags: review?(florian)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8354597 [details] [diff] [review] Patch 2 *** Original change on bio 2089 attmnt 2827 at 2013-09-06 05:48:56 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354597 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
Comment on attachment 8354605 [details] [diff] [review] Patch 3 *** Original change on bio 2089 attmnt 2835 at 2013-09-06 10:27:59 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354605 -
Flags: review?(florian) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 10•10 years ago
|
||
*** Original post on bio 2089 at 2013-09-09 23:35:41 UTC *** Thanks! http://hg.instantbird.org/instantbird/rev/7bdbfa0b739f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•