openwisp-firmware-upgrader
openwisp-firmware-upgrader copied to clipboard
[bug] Opening change page for device without DeviceConnection object logs error
get_upgrader_class_from_device_connection logs an exception when the conn is None
'NoneType' object has no attribute 'update_strategy'
Traceback (most recent call last):
File "/opt/openwisp2/env/lib/python3.10/site-packages/openwisp_firmware_upgrader/utils.py", line 36, in get_upgrader_class_from_device_connection
upgrader_class = app_settings.UPGRADERS_MAP[device_conn.update_strategy]
AttributeError: 'NoneType' object has no attribute 'update_strategy'
We need to investigate what code is calling this function with None argument.
https://github.com/openwisp/openwisp-firmware-upgrader/blob/f1b0397944489db857cde986c2933675b3758001/openwisp_firmware_upgrader/utils.py#L34C5-L41
Steps to replicate
- Create a device
- Ensure that this device does not have a credentials object. If the device has one (due to auto_add), remove the credential and reload the page.
Actual Outcome
An AttributeError is logged because the form generation requires schema of the upgrader class:
https://github.com/openwisp/openwisp-firmware-upgrader/blob/f1b0397944489db857cde986c2933675b3758001/openwisp_firmware_upgrader/admin.py#L422-L427
Since this device does not have a DeviceConnection object, the get_upgrader_schema_for_device calls the get_upgrader_class_from_device_connection function with None argument through get_upgrader_class_for_device.
'NoneType' object has no attribute 'update_strategy'
Traceback (most recent call last):
File "/home/pandafy/openwisp/openwisp-firmware-upgrader/openwisp_firmware_upgrader/utils.py", line 36, in get_upgrader_class_from_device_connection
upgrader_class = app_settings.UPGRADERS_MAP[device_conn.update_strategy]
AttributeError: 'NoneType' object has no attribute 'update_strategy'
I see an opportunity for improving UX here. This is how the form looks when the upgrader class schema is not available
There are no options for the upgrade because the DeviceConnection.update_strategy is used for getting the correct upgrader class which in-turns provide the schema.
In this case, if the user choose a FirmwareImage and go ahead anyway, they will see an error on submitting the form
I think, we can use this exception to pro-actively tell users to add a credential first.
Seems a good plan. I think for "DEVICE CONNECTIONS" you meant credentials?
Yes, we should also update that error message.