Closed
Bug 955667
Opened 11 years ago
Closed 11 years ago
Unhandled IRC message 263: LIST refused
Categories
(Chat Core :: IRC, defect)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: aleth)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2220 at 2013-10-15 16:49:00 UTC ***
Timestamp: 10/15/2013 06:43:24 PM
Warning: Unhandled IRC message:
{"rawMessage":":dickson.freenode.net 263 aleth LIST :This command could not be completed because it has been used recently, and is rate-limited.","command":"263","params":["aleth","LIST","This command could not be completed because it has been used recently, and is rate-limited."],"servername":"dickson.freenode.net"}
Source File: resource://gre/components/irc.js
Line: 692
Source Code:
prpl-irc
We should handle this response to LIST.
Assignee | ||
Comment 1•11 years ago
|
||
*** Original post on bio 2220 as attmnt 2950 at 2013-10-15 18:36:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354731 -
Flags: review?(clokep)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
*** Original post on bio 2220 at 2013-10-15 18:38:02 UTC ***
nhnt11: Is there a way to tell the callback receivers that LIST failed?
Comment 3•11 years ago
|
||
Comment on attachment 8354731 [details] [diff] [review]
Patch
*** Original change on bio 2220 attmnt 2950 at 2013-10-15 18:51:20 UTC ***
Can you add a comment about when this happens for list or something. I doubt you want to do the server message always for list, by the way. If it was from a command we probably want to print out a message, otherwise...I'm not sure what to do.
Attachment #8354731 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 4•11 years ago
|
||
*** Original post on bio 2220 at 2013-10-15 18:54:16 UTC ***
(In reply to comment #3)
> I doubt you want to do the server message always for list, by the way. If it
> was from a command we probably want to print out a message, otherwise...I'm
> not sure what to do.
How soon after receiving this LIST failure message do you think we should retry? (nhnt11 thinks 12h is too long in this case).
Assignee | ||
Comment 5•11 years ago
|
||
*** Original post on bio 2220 as attmnt 2952 at 2013-10-15 19:59:00 UTC ***
Improve comment, set last list time in such a way that we may retry after an hour rather than 12.
Attachment #8354733 -
Flags: review?(clokep)
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 8354731 [details] [diff] [review]
Patch
*** Original change on bio 2220 attmnt 2950 at 2013-10-15 19:59:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354731 -
Attachment is obsolete: true
Comment 7•11 years ago
|
||
Comment on attachment 8354733 [details] [diff] [review]
Patch
*** Original change on bio 2220 attmnt 2952 at 2013-10-15 20:07:44 UTC ***
This is pretty clearly untested.
>diff --git a/chat/protocols/irc/ircBase.jsm b/chat/protocols/irc/ircBase.jsm
> "263": function(aMessage) { // RPL_TRYAGAIN
> // <command> :Please wait a while and try again.
>- // TODO setTimeout for a minute or so and try again?
>- return false;
>+ if (aMessage.params[1] == "LIST" && this._pendingList) {
What does pendingList mean in this context?
>+ // We may receive this from servers which rate-limit LIST if the
>+ // server believes us to be asking for LIST data to soon after the
Typo: "too soon"
>+ // previous request.
>+ // Tidy up as we won't be receiving any more channels.
>+ this._sendRemainingRoomInfo();
>+ // Fake the last LIST time so that we may try again in an hour.
Be explicit and say "one hour" please.
>+ return serverMessage(this, aMessage);
Should this return false as previously?
>diff --git a/chat/protocols/irc/ircUtils.jsm b/chat/protocols/irc/ircUtils.jsm
>+const kListRefreshInterval: 12 * 60 * 60 * 1000; // 12 hours.
I think you mean =, not :. This needs to be exported if you want access to it in other places. This should probably have a comment above it also.
Attachment #8354733 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 8•11 years ago
|
||
*** Original post on bio 2220 at 2013-10-15 20:15:42 UTC ***
>>+ if (aMessage.params[1] == "LIST" && this._pendingList) {
> What does pendingList mean in this context?
It means that the user has used /list or the newtab service has sent
LIST. Therefore there are callbacks etc we need to tidy up.
>>+ // Fake the last LIST time so that we may try again in an hour.
> Be explicit and say "one hour" please.
No, it's not going to happen again in one hour automatically. It'll only
happen if after one hour
someone opens a newtab...
>>+ return serverMessage(this, aMessage);
> Should this return false as previously?
I think showing it in the server tab is better than not handling it at
all.
>>+const kListRefreshInterval: 12 * 60 * 60 * 1000; // 12 hours.
Thanks for catching this! The patch was untested as freenode is suddenly
allowing me to LIST again... not a good excuse ;)
Assignee | ||
Comment 9•11 years ago
|
||
*** Original post on bio 2220 as attmnt 2954 at 2013-10-15 20:33:00 UTC ***
Should address the review comments. Tested in the "doesn't break IRC" sense.
Attachment #8354735 -
Flags: review?(clokep)
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 8354733 [details] [diff] [review]
Patch
*** Original change on bio 2220 attmnt 2952 at 2013-10-15 20:33:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354733 -
Attachment is obsolete: true
Comment 11•11 years ago
|
||
Comment on attachment 8354735 [details] [diff] [review]
Patch
*** Original change on bio 2220 attmnt 2954 at 2013-10-16 13:06:34 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354735 -
Flags: review?(clokep) → review+
Updated•11 years ago
|
Whiteboard: [checkin-needed]
Comment 12•11 years ago
|
||
*** Original post on bio 2220 at 2013-10-18 11:14:23 UTC ***
http://hg.instantbird.org/instantbird/rev/5ae435057c7a
Status: ASSIGNED → RESOLVED
Closed: 11 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
•