Closed
Bug 955250
Opened 10 years ago
Closed 10 years ago
Don't disconnect when failing to parse a message
Categories
(Chat Core :: IRC, defect)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.4
People
(Reporter: aleth, Assigned: clokep)
Details
Attachments
(1 file, 1 obsolete file)
3.91 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1817 at 2012-11-22 14:12:00 UTC *** This has come up a couple of times, most recently in the bug reported by qlum.
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1817 at 2012-11-22 14:15:31 UTC *** (Looks like that bug wasn't filed, so I added bug 955251 (bio 1818))
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1817 as attmnt 2124 at 2012-11-22 18:37:00 UTC *** I think this should fix the issue...
Attachment #8353885 -
Flags: feedback?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8353885 [details] [diff] [review] Untested patch *** Original change on bio 1817 attmnt 2124 at 2012-11-22 18:40:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353885 -
Flags: feedback? → feedback?(aleth)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8353885 [details] [diff] [review] Untested patch *** Original change on bio 1817 attmnt 2124 at 2012-11-22 19:23:28 UTC *** This looks like it should do the trick, but it does really need testing ;) I don't think we should log an unhandled message warning when we couldn't parse the message in the first place, so I'd suggest moving the if (!handled) inside the try.
Attachment #8353885 -
Flags: feedback?(aleth) → feedback+
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 1817 at 2012-11-23 04:03:15 UTC *** (In reply to comment #3) > Comment on attachment 8353885 [details] [diff] [review] (bio-attmnt 2124) [details] > Untested patch > > This looks like it should do the trick, but it does really need testing ;) I agree! I'll see if I can figure out a way to test it soon... > I don't think we should log an unhandled message warning when we couldn't parse > the message in the first place, so I'd suggest moving the if (!handled) inside > the try. You're probably right, we're already displaying the error in that case saying it couldn't be parsed. Thanks!
Reporter | ||
Comment 6•10 years ago
|
||
*** Original post on bio 1817 at 2012-11-23 14:33:03 UTC *** (In reply to comment #4) > > This looks like it should do the trick, but it does really need testing ;) > I agree! I'll see if I can figure out a way to test it soon... Intentionally breaking the regex should do the trick. > > I don't think we should log an unhandled message warning when we couldn't parse > > the message in the first place, so I'd suggest moving the if (!handled) inside > > the try. > You're probably right, we're already displaying the error in that case saying > it couldn't be parsed. Thanks! On second thoughts, would that mean we no longer get the message displayed if there is an error in some message handler?
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1817 as attmnt 2130 at 2012-11-25 04:55:00 UTC *** I tested this by making the regexp not match an IRC message.
Attachment #8353891 -
Flags: review?(aleth)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8353885 [details] [diff] [review] Untested patch *** Original change on bio 1817 attmnt 2124 at 2012-11-25 04:55:43 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353885 -
Attachment is obsolete: true
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8353891 [details] [diff] [review] Patch *** Original change on bio 1817 attmnt 2130 at 2012-11-25 13:41:31 UTC *** Tested it for errors in handlers and it works great :) Thanks for this improvement.
Attachment #8353891 -
Flags: review?(aleth) → review+
Reporter | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 10•10 years ago
|
||
*** Original post on bio 1817 at 2012-11-25 23:38:41 UTC *** Committed as http://hg.instantbird.org/instantbird/rev/e2ff32e19c78
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.4
You need to log in
before you can comment on or make changes to this bug.
Description
•