Closed Bug 955700 Opened 8 years ago Closed 8 years ago

Accounts stuck in the "Connecting..." state

Categories

(Chat Core :: General, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: aleth)

References

Details

(Whiteboard: [1.5-blocking][regression])

Attachments

(1 file, 1 obsolete file)

10.46 KB, patch
florian
: review+
clokep
: review+
Details | Diff | Splinter Review
*** Original post on bio 2251 at 2013-11-15 11:13:00 UTC ***

This is how the debug log looks:
[14/11/13 21:38:57] ERROR (@ prpl-irc resource://gre/components/irc.js:722)
Connection closed by server.
[14/11/13 21:38:57] LOG   (@ prpl-irc resource:///modules/socket.jsm:181)
Disconnect
[14/11/13 21:38:58] LOG   (@ prpl-irc resource:///modules/socket.jsm:145)
Connecting to: chat.freenode.net:6667
[14/11/13 21:38:58] LOG   (@ prpl-irc resource:///modules/socket.jsm:330)
using socks proxy: localhost:1080
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_RESOLVING)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_RESOLVED)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:524)
onTransportStatus(STATUS_CONNECTING_TO)
[14/11/13 21:38:58] DEBUG (@ prpl-irc resource:///modules/socket.jsm:447)
onStartRequest
Whiteboard: [1.5-blocking][possible regression]
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 2251 as attmnt 3052 at 2013-11-15 13:23:00 UTC ***

This patch will fix it if the issue is that we are receiving a stopRequest in response to the timeout when we are not yet connected.
Attachment #8354833 - Flags: review?(florian)
Comment on attachment 8354833 [details] [diff] [review]
Patch

*** Original change on bio 2251 attmnt 3052 at 2013-11-15 22:34:08 UTC ***

Thanks for looking into this. You are correct about what's going on, and the patch fixes the issue.

I'm not r+'ing right now because I'm wondering if we shouldn't replace the isConnected (and potential isConnecting) by isDisconnected. Thoughts?
Attachment #8354833 - Flags: feedback+
Blocks: 955676
Whiteboard: [1.5-blocking][possible regression] → [1.5-blocking][regression]
*** Original post on bio 2251 at 2013-11-16 17:23:23 UTC ***

(In reply to comment #2) 
> I'm not r+'ing right now because I'm wondering if we shouldn't replace the
> isConnected (and potential isConnecting) by isDisconnected. Thoughts?

This makes sense, *but* isConnected is part of the API, and I think we currently use it, e.g. http://lxr.instantbird.org/instantbird/source/chat/protocols/yahoo/yahoo-session.jsm#380.
*** Original post on bio 2251 at 2013-11-16 19:42:39 UTC ***

We also use it in IRC:
http://lxr.instantbird.org/instantbird/source/chat/protocols/irc/irc.js#1387
http://lxr.instantbird.org/instantbird/source/chat/protocols/irc/irc.js#1529

Both of these check !isConnected (which should be the same as isDisconnected). If you're very concerned about compatibility...it's possible you could use isDisconnected, then have a getter that is isConnected() !isDisconnected, although that's not entirely true.
Comment on attachment 8354833 [details] [diff] [review]
Patch

*** Original change on bio 2251 attmnt 3052 at 2013-11-16 22:41:15 UTC ***

Let's use "disconnected" here instead of isConnected and isConnecting.
Attachment #8354833 - Flags: review?(florian) → review-
Attached patch PatchSplinter Review
*** Original post on bio 2251 as attmnt 3060 at 2013-11-18 16:39:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354841 - Flags: review?(florian)
Comment on attachment 8354833 [details] [diff] [review]
Patch

*** Original change on bio 2251 attmnt 3052 at 2013-11-18 16:39:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354833 - Attachment is obsolete: true
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment on attachment 8354841 [details] [diff] [review]
Patch

*** Original change on bio 2251 attmnt 3060 at 2013-11-18 16:44:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354841 - Flags: review?(clokep)
Comment on attachment 8354841 [details] [diff] [review]
Patch

*** Original change on bio 2251 attmnt 3060 at 2013-11-18 16:54:21 UTC ***

Seems reasonable to me, but I would also like clokep to have a look.
Attachment #8354841 - Flags: review?(florian) → review+
Comment on attachment 8354841 [details] [diff] [review]
Patch

*** Original change on bio 2251 attmnt 3060 at 2013-11-18 16:59:04 UTC ***

I didn't notice anything scary in it.
Attachment #8354841 - Flags: review?(clokep) → review+
OS: Other → All
Hardware: x86 → All
Whiteboard: [1.5-blocking][regression] → [1.5-blocking][regression][checkin-needed]
*** Original post on bio 2251 at 2013-11-22 07:40:59 UTC ***

Sorry the checkin took so long for no reason... which is especially strange given how annoying the bug is (was? Thanks for fixing it! :-)).

http://hg.instantbird.org/instantbird/rev/0e870bcb05b7
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [1.5-blocking][regression][checkin-needed] → [1.5-blocking][regression]
Target Milestone: --- → 1.5
You need to log in before you can comment on or make changes to this bug.