openwisp-firmware-upgrader icon indicating copy to clipboard operation
openwisp-firmware-upgrader copied to clipboard

[bug] Opening change page for device without DeviceConnection object logs error

Open pandafy opened this issue 1 year ago • 4 comments
trafficstars

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

pandafy avatar Feb 16 '24 12:02 pandafy

Steps to replicate

  1. Create a device
  2. 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'

pandafy avatar Feb 16 '24 13:02 pandafy

I see an opportunity for improving UX here. This is how the form looks when the upgrader class schema is not available Screenshot from 2024-02-16 18-42-54

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

Screenshot from 2024-02-16 18-52-30

I think, we can use this exception to pro-actively tell users to add a credential first.

pandafy avatar Feb 16 '24 13:02 pandafy

Seems a good plan. I think for "DEVICE CONNECTIONS" you meant credentials?

nemesifier avatar Feb 16 '24 13:02 nemesifier

Yes, we should also update that error message.

pandafy avatar Feb 16 '24 13:02 pandafy