Skip to content

Conversation

@pandafy
Copy link
Member

@pandafy pandafy commented May 1, 2025

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • N/A I have updated the documentation.

Description of Changes

Bug:
When multiple connections are made to the websocket command endpoint of the same device (e.g. when multiple browser tabs are open for the same device), the UI does not receive updates from the websocket and keeps showing a loader in the command output field, even when the command has completed execution.

Fix:
The issue was caused by mutating the shared event dictionary in the send_update method of CommandConsumer. Specifically, calling event.pop('type') removed the 'type' key from the event. Since the same event object is dispatched to all consumer instances (one for each websocket connection), removing 'type' in one instance caused the others to raise:

ValueError: Incoming message has no 'type' attribute

This broke message dispatch for the remaining connections. The fix is to avoid modifying the original event dictionary.
This preserves the 'type' key, ensuring all consumer instances continue to receive well-formed events and function correctly.

Bug:
When multiple connections are made to the websocket command
endpoint of the same device (e.g. when multiple browser tabs are open for
the same device), the UI does not receive updates from the websocket and
keeps showing a loader in the command output field, even when the command
has completed execution.

Fix:
The issue was caused by mutating the shared `event` dictionary in the
`send_update` method of `CommandConsumer`. Specifically, calling
`event.pop('type')` removed the `'type'` key from the event. Since the
same event object is dispatched to all consumer instances (one for each
websocket connection), removing `'type'` in one instance caused the others
to raise:

    ValueError: Incoming message has no 'type' attribute

This broke message dispatch for the remaining connections. The fix is to
avoid modifying the original `event` dictionary.
This preserves the `'type'` key, ensuring all consumer instances
continue to receive well-formed events and function correctly.
@pandafy pandafy moved this from To do (general) to Needs review in OpenWISP Contributor's Board May 1, 2025
@coveralls
Copy link

Coverage Status

coverage: 98.868%. remained the same
when pulling 95a9d37 on fix-send-command-updates
into 1745a0a on master.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 👍

@github-project-automation github-project-automation bot moved this from Needs review to In progress in OpenWISP Contributor's Board May 8, 2025
@nemesifier nemesifier merged commit 35dbfe5 into master May 8, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in OpenWISP Contributor's Board May 8, 2025
@nemesifier nemesifier deleted the fix-send-command-updates branch May 8, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants