GNOME Bugzilla – Bug 658150
network: apObj is null exceptions
Last modified: 2011-10-29 14:19:58 UTC
My high level goal here was to debug "apObj is null" errors, and while doing that I noticed We had two near-exact copies of the code to add an access point: 1) At object creation time 2) Later A straightforward fix by just having 1) call 2) doesn't work because in 2) we attempt to update the UI, but the object isn't fully constructed yet - we call the superclass constructor at the end of the subclass one =( This is a pretty conceptually broken thing to do, but undoing it is not trivial. So for now, split the access point addition code into two functions, one which updates the core data structures, and the other which calls the first and also updates the UI.
Created attachment 195605 [details] [review] network: Remove duplicated access-point addition code
Review of attachment 195605 [details] [review]: ::: js/ui/status/network.js @@ +954,3 @@ // this.connectionValid until I have a device this.device = device; + this._connections = []; This is not enough: this._connections stores the NMRemoteConnections wrapped in a small object with timestamp, name and uuid, and _accessPointAdded expects this wrapping. (as we also store that as JS properties on the NMRemoteConnection object, we could kill the wrapper object). Plus, until we chain to parent class, this array is empty, so your addAccessPointCore will fail to associate networks with existing connections. On the other hand, until you fill _networks, createSection does nothing, so you need to chain at the end. @@ +1174,3 @@ apObj = this._networks[pos]; if (apObj.accessPoints.indexOf(accessPoint) != -1) { + return apObj; Why this change? If the access point was already associated with the network, we should not update the UI. (In fact, this should never happen, as libnm-glib should not emit access-point-added twice for the same object)
Hmm. Do you have any suggestions for alternative changes? Do you think you might be able to do the refactoring to call the superclass constructor at the start?
For reference, here's the current exceptions I'm seeing: JS ERROR: !!! Exception was: Error: Expected type void for Argument 'ssid' but got type 'object' (nil) JS ERROR: !!! lineNumber = '0' JS ERROR: !!! fileName = '"gjs_throw"' JS ERROR: !!! stack = '"("Expected type void for Argument 'ssid' but got type 'object' (nil)")@gjs_throw:0 ssidToLabel(null)@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:95 ([object Array])@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:116 NMNetworkMenuItem([object Array])@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:102 ([object Object],0)@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:1494 ()@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:1539 ([object _private_NMClient_DeviceWifi],30,20,42)@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:634 "' JS ERROR: !!! message = '"Expected type void for Argument 'ssid' but got type 'object' (nil)"' JS ERROR: !!! message = '"apObj is null"' JS ERROR: !!! Exception was: TypeError: this.bestAP is undefined JS ERROR: !!! lineNumber = '115' JS ERROR: !!! fileName = '"/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js"' JS ERROR: !!! stack = '"([object Array])@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:115 NMNetworkMenuItem([object Array])@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:102 ([object Object],0)@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:1494 ()@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:1539 ([object _private_NMClient_DeviceWifi],30,20,42)@/src/build/jhbuild/share/gnome-shell/js/ui/status/network.js:634 "' JS ERROR: !!! message = '"this.bestAP is undefined"'
One of these exceptions seemed to "wedge" the shell network UI. This is pretty bad. Nominating as a 3.2 blocker.
Created attachment 196877 [details] [review] Don't create AP items for empty networks Current code is sometime attempting to create menu items for wifi networks that have no visible AP. I have no idea why this is happening, but it should fix the symptoms and avoid exceptions.
Created attachment 196878 [details] [review] Clear the active network when removing the active access point When the active AP disappears, it is possible to receive the "access-point-removed" signal before the "notify::active-ap" (as dbus-glib + libnm-glib property notifications are not reliable). In that case, we would remove the AP from the network object, thus an attempt to update the UI would create an item for an empty network. This is not a fix for the exceptions you're seeing (the could would call the failing new NMNetworkMenuItem from a different point), but it should help anyway.
Review of attachment 196878 [details] [review]: This change looks good to me.
Review of attachment 196877 [details] [review]: One minor comment, otherwise - OK I guess. ::: js/ui/status/network.js @@ +1483,3 @@ + if(!apObj.accessPoints || apObj.accessPoints.length == 0) + // this should not happen, but I have no idea why it happens + return; Can you add { } around this comment + statement? While it works now it's a trap for someone who comes along later and adds another line in here - the fact that it's two lines makes it less obvious we're using the one-statement shortcut.
Attachment 196877 [details] pushed as 05e0d50 - Don't create AP items for empty networks Attachment 196878 [details] pushed as 82fc663 - Clear the active network when removing the active access point
I'm still seeing these exceptions. JS ERROR: !!! message = '"apObj is null"' JS ERROR: !!! Exception was: TypeError: apObj is null JS ERROR: !!! lineNumber = '1305' JS ERROR: !!! fileName = '"/home/jstpierre/Source/gnome3/install/share/gnome-shell/js/ui/status/network.js"' JS ERROR: !!! stack = '"([object _private_NMClient_DeviceWifi],[object _private_NMClient_AccessPoint])@/home/jstpierre/Source/gnome3/install/share/gnome-shell/js/ui/status/network.js:1305
(In reply to comment #12) > I'm still seeing these exceptions. > > JS ERROR: !!! message = '"apObj is null"' > JS ERROR: !!! Exception was: TypeError: apObj is null > JS ERROR: !!! lineNumber = '1305' > JS ERROR: !!! fileName = > '"/home/jstpierre/Source/gnome3/install/share/gnome-shell/js/ui/status/network.js"' > JS ERROR: !!! stack = '"([object _private_NMClient_DeviceWifi],[object > _private_NMClient_AccessPoint])@/home/jstpierre/Source/gnome3/install/share/gnome-shell/js/ui/status/network.js:1305 Duplicate of byg 659277 (the menu is empty but not cleared).
Yeah, my bad. I found the correct bug.