GNOME Bugzilla – Bug 646355
Cope better with invalid or unsupported connections
Last modified: 2011-04-03 12:25:30 UTC
Currently connections that are of unsupported type (like wimax, or like those emitted by NM up until NetworkManager/b8a1a3864005307929632d77575bcde92b3354c5) throw exceptions that cause the menu to end up completely emtpy. Instead, those should be handled before throwing (and eventually supported, but that's after 3.2).
Created attachment 184796 [details] [review] NetworkStatus: ignore broken connections Sometimes NetworkManager sends connections that have invalid settings. This should not happen, but in case it does, we should not blow up, but just log a warning and continue silently.
Comment on attachment 184796 [details] [review] NetworkStatus: ignore broken connections do we need to remove the connection from _connections in this case? Otherwise, it seems like weird things could happen; eg, places that check this._connections.length might do the wrong thing.
Created attachment 184803 [details] [review] NetworkStatus: ignore broken and unsupported connections Sometimes NetworkManager sends connections that have invalid settings. This should not happen, but in case it does, we should not blow up, but just log a warning and continue silently. Also, some connection types (like wimax) are just not supported by the menu, and should be ignored as well. Updated patch. I think that we need to continue tracking all connections in _connections and _updateConnection, because _syncActiveConnection will retrieve the connection directly from libnm-glib and code wants _name, _type, _section and _uuid to be always present and valid.
Comment on attachment 184803 [details] [review] NetworkStatus: ignore broken and unsupported connections cool, much simpler. You still need to disconnect the signal handlers in _connectionRemoved though I think?
If you don't mind, could you update the patch description to say "invalid or unsupported", since stuff like wimax or pppoe certainly isn't valid, but is currently unsupported by the indicator.
Review of attachment 184803 [details] [review]: Agree with danw that you are missing the signal connection removal (though I don't think that will cause any actual problems), otherwise looks OK to me.
Created attachment 184953 [details] [review] NetworkStatus: ignore invalid and unsupported connection types Some connection types (like wimax) are not supported by the menu, and should be ignored instead of throwing exceptions. Also, NetworkManager had a bug that sent connections with invalid settings. This should not happen, but in case it does, we will not blow up, but just log a warning and continue silently.
Review of attachment 184953 [details] [review]: Patch looks right to me. Tested by hacking the code to make VPN an unrecognized type and without the patch, empty menu (Network Setttings only) and with the patch came up correctly just without the VPN connections. Good pending R-T approval.
Attachment 184953 [details] pushed as 20f1457 - NetworkStatus: ignore invalid and unsupported connection types