GNOME Bugzilla – Bug 771217
Fix another connection issue
Last modified: 2016-09-23 23:44:15 UTC
So it turns out I lost another connection parameter again, this time for Freenode. I figured out the issue however, see the first patch. What made the issue more pressing was that it didn't fix itself by retrying with alternate servers from the list, as all entries only differ by port and we end up filtering them all out. The last patch addresses this issue as well.
Created attachment 335279 [details] [review] connections: Always set port when SSL is enabled At least for now telepathy-idle does not consider the 'use-ssl' parameter for picking a default port, so include it even when not explicitly specified to make sure it is not accidentally removed.
Created attachment 335280 [details] [review] connections: Factor out getAccountParams() method There are cases where we don't just want to retrieve and unpack account parameters, but also make sure that appropriate fallbacks for missing values are in place; split out an appropriate help function for that.
Created attachment 335281 [details] [review] application: Consider port for alternate server list When we fail to connect to a network with multiple servers, we build a list of alternate servers for retrying, excluding the original server that already failed. Currently that filtering is based solely on the server address, in the assumption that servers would not just differ by port - however that's exactly the case for FreeNode, so to not effectively disable connection retries there, we need to take the port number into account as well.
Review of attachment 335280 [details] [review]: + params[p] = params[p].deep_unpack(); + + params['use-ssl'] = !!params['use-ssl']; Is there a typo here? I was unable to find the '!!' operator.
(In reply to Rares Visalom from comment #4) > + params['use-ssl'] = !!params['use-ssl']; > > Is there a typo here? I was unable to find the '!!' operator. No, it's a C-ism that is occasionally useful. '!!' is not an operator on its own, it's the '!' operator applied twice. The result is a proper boolean, even when the original value was only falsey or trueish (for example null, 1, undefined, '', ...). In this case, it's to handle the case where 'use-ssl' isn't defined, and could alternatively be written like: params['use-ssl'] = params['use-ssl'] || false;
Review of attachment 335279 [details] [review]: looks good to me.
Review of attachment 335280 [details] [review]: looks good to me.
Review of attachment 335280 [details] [review]: (forgot to change status)
Review of attachment 335281 [details] [review]: looks good to me.
Attachment 335279 [details] pushed as fbb168b - connections: Always set port when SSL is enabled Attachment 335280 [details] pushed as 1a23da2 - connections: Factor out getAccountParams() method Attachment 335281 [details] pushed as a2331c7 - application: Consider port for alternate server list