Skip to content

Fix KeyError in ROS2 ActionClient when receiving action results from rosbridge - #142

Merged
gonzalocasas merged 4 commits into
mainfrom
copilot/fix-eb4017ce-00dc-4df3-994b-8484ca35ba21
Oct 7, 2025
Merged

gonzalocasas merged 4 commits into
mainfrom
copilot/fix-eb4017ce-00dc-4df3-994b-8484ca35ba21

Conversation

Copilot AI commented Sep 30, 2025 •

Copy link
Copy Markdown
Contributor

Summary

Fixed the KeyError that occurred when ROS2 ActionClient received action results from rosbridge. The issue was caused by the code attempting to access message["status"] without checking if the field existed or handling different message formats.

Changes

  • src/roslibpy/comm/comm.py: Modified _handle_action_result() to handle multiple message formats (status at top level or inside values)
  • tests/ros2/test_action_client.py: Added comprehensive test suite with 4 test cases covering all scenarios
  • CHANGELOG.rst: Documented the fix in the Unreleased section
  • Linter fixes: Applied black and isort formatting to fix linter errors

Testing

✅ All 4 new tests pass
✅ All existing unit tests still pass
✅ All linters pass (flake8, black, isort)

Impact

  • Minimal code changes (only 12 lines modified)
  • Backward compatible with existing ROS1 message format
  • Forward compatible with ROS2 message variations
  • Defensive handling of edge cases
Original prompt

This section details on the original issue you should resolve

<issue_title>ROS2 ActionClient fails with KeyError when receiving action results from rosbridge</issue_title>
<issue_description>Description

ROS2 ActionClient fails with KeyError when receiving action results from rosbridge

To Reproduce

python
import roslibpy
from roslibpy import ActionClient

client = roslibpy.Ros(host='localhost', port=9090)
client.run()

action = ActionClient(
    client,
    '/do_objective',
    'moveit_studio_sdk_msgs/action/DoObjectiveSequence'
)

def on_result(result):
    print(f"Result: {result}")

def on_feedback(feedback):
    print(f"Feedback: {feedback}")

def on_error(error):
    print(f"Error: {error}")

goal = {"objective_name": "test"}
goal_id = action.send_goal(goal, on_result, on_feedback, on_error)

# Wait for result...

Error Output

Exception on start_listening while trying to handle message received.It could indicate a bug in user code on message handlers. Message skipped.
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/dist-packages/roslibpy/comm/comm_autobahn.py", line 40, in onMessage
    self.on_message(payload)
  File "/usr/local/lib/python3.12/dist-packages/roslibpy/comm/comm.py", line 49, in on_message
    handler(message)
  File "/usr/local/lib/python3.12/dist-packages/roslibpy/comm/comm.py", line 166, in _handle_action_result
    LOGGER.debug("Received Action result with status: %s", message["status"])
                                                           ~~~~~~~^^^^^^^^^^
KeyError: 'status'

Expected behavior
The message structure from rosbridge is:

{
  "values": {
    "status": 4,    // <-- Status is HERE
    "result": {...}
  }
}

But roslibpy expects:
message["status"] # <-- Expects it at top level

The ActionClient's result callback should receive the action result
without crashing. The handler should properly parse the action result
message from rosbridge and pass the result data to the user's callback
function.

Screenshots

System (please complete the following information):

  • OS: Ubuntu 24.04
    • Python version 3.12
    • Python package manager pip
    • roslibpy version: main branch (commit 1d549b7)
    • rosbridge_suite version: ros2 branch
    • ROS version: ROS2 Humble

Additional context
The bug occurs at line 166 in src/roslibpy/comm/comm.py where the handler
tries to access message["status"] for logging before verifying it
exists. The rosbridge protocol for ROS2 actions sends:

{
   "op": "action_result",
   "action": "/action_name",
   "id": "send_action_goal:/action_name:1",
   "status": 3,
   "values": {...},
   "result": true
 }

Current workaround that users can apply:

from roslibpy.comm import comm
 def fix(self, msg):
     handlers = self._pending_action_requests.pop(msg["id"], None)
     if handlers and msg.get("result"):
         handlers[0]({"status": msg.get("status"), "values":
 msg.get("values", {})})
     elif handlers:
         handlers[2]("Action failed")
 comm.RosBridgeProtocol._handle_action_result = fix

</issue_description>

<agent_instructions>Write a test in the ROS2 test suite that reproduces this problem, and then proceed to fix it</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #141

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits September 30, 2025 08:51
Co-authored-by: gonzalocasas <933277+gonzalocasas@users.noreply.github.com>
Co-authored-by: gonzalocasas <933277+gonzalocasas@users.noreply.github.com>
Copilot AI changed the title [WIP] ROS2 ActionClient fails with KeyError when receiving action results from rosbridge Fix KeyError in ROS2 ActionClient when receiving action results from rosbridge Sep 30, 2025
Copilot AI requested a review from gonzalocasas September 30, 2025 08:58
@gonzalocasas

Copy link
Copy Markdown
Contributor

@copilot fix the linter errors

Co-authored-by: gonzalocasas <933277+gonzalocasas@users.noreply.github.com>

Copilot AI commented Sep 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot fix the linter errors

Fixed in commit 631d567. Applied black and isort formatting to both files - all linters now pass.

@gonzalocasas
gonzalocasas marked this pull request as ready for review September 30, 2025 12:09
@gonzalocasas

Copy link
Copy Markdown
Contributor

@noah-wardlow could you please take a look at this and confirm if it fixes your problem?

@noah-wardlow

Copy link
Copy Markdown

@gonzalocasas can confirm this fix worked for me

@gonzalocasas gonzalocasas left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified by reporter

@gonzalocasas

Copy link
Copy Markdown
Contributor

@sea-bass @EzraBrooks could you guys take a look at this and review? the merge rules from me from merging because I collaborated with Copilot ;)

@gonzalocasas gonzalocasas mentioned this pull request Oct 6, 2025
5 of 9 tasks
@gonzalocasas
gonzalocasas merged commit 97e8d5f into main Oct 7, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ROS2 ActionClient fails with KeyError when receiving action results from rosbridge

3 participants