Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
pandafy opened this issue Feb 16, 2024 · 4 comments · Fixed by #252
Closed

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

pandafy opened this issue Feb 16, 2024 · 4 comments · Fixed by #252
Labels
bug Something isn't working

Comments

@pandafy
Copy link
Member

pandafy commented Feb 16, 2024

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 pandafy added the bug Something isn't working label Feb 16, 2024
@pandafy pandafy changed the title [bug] Do not log exception if the device does not has a device connection [bug] Investigate why get_upgrader_class_from_device_connection is getting called with None argument Feb 16, 2024
@pandafy
Copy link
Member Author

pandafy commented Feb 16, 2024

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:

def get_formset(self, request, obj=None, **kwargs):
formset = super().get_formset(request, obj=obj, **kwargs)
if obj:
schema = get_upgrader_schema_for_device(obj)
formset.extra_context = json.dumps(schema, cls=DjangoJSONEncoder)
return formset

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
Copy link
Member Author

pandafy commented Feb 16, 2024

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.

@nemesifier
Copy link
Member

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

@pandafy
Copy link
Member Author

pandafy commented Feb 16, 2024

Yes, we should also update that error message.

@pandafy pandafy changed the title [bug] Investigate why get_upgrader_class_from_device_connection is getting called with None argument [bug] Opening change page for device with DeviceConnection object logs error Feb 16, 2024
@pandafy pandafy changed the title [bug] Opening change page for device with DeviceConnection object logs error [bug] Opening change page for device without DeviceConnection object logs error Feb 16, 2024
pandafy added a commit that referenced this issue Apr 29, 2024
Don't log error on device change page if the device does not
have any DeviceConnection objects.

Closes #249
pandafy added a commit that referenced this issue Apr 29, 2024
Don't log error on device change page if the device does not
have any DeviceConnection objects.

Closes #249
nemesifier pushed a commit that referenced this issue Apr 30, 2024
Don't log error on device change page if the device does not
have any DeviceConnection objects.

Closes #249
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
2 participants