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

Fix error reporting TCPROSServer #2299

Open
wants to merge 1 commit into
base: noetic-devel
Choose a base branch
from

Conversation

g-gemignani
Copy link
Contributor

@g-gemignani g-gemignani commented Nov 18, 2022

Stumbled across this little bug while hunting for another one

@g-gemignani g-gemignani force-pushed the gu/fix_error_reporting_TCPROSServer branch from b5f888d to 9d7b6d2 Compare November 18, 2022 10:02
@mjcarroll
Copy link
Member

Can you describe the bug that this is fixing please?

@peci1
Copy link
Contributor

peci1 commented Apr 9, 2023

@g-gemignani The fix looks reasonable, but I agree with Michael a little context would help. In what situation did you face an error? How did it manifest?

@g-gemignani
Copy link
Contributor Author

Hi and thank you for the answer. Yes, you are right, I was in a rush when I opened the fix and did not attach more info. Here it is:
I was debugging a python interactive CLI that was spawning ros nodes in child processes upon issuing commands. The library was using multiprocessing (https://docs.python.org/3/library/multiprocessing.html).
The issue that I was getting was a cascade of:

Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes

After digging a bit, I found out that the issue was in ros_comm:

Traceback (most recent call last):
File "/home/toru/catkin_ws/install/lib/python3/dist-packages/rospy/impl/tcpros_base.py", line 350, in _tcp_server_callback
write_ros_handshake_header(sock, {'error' : err_msg})
File "/home/toru/catkin_ws/install/lib/python3/dist-packages/rosgraph/network.py", line 416, in write_ros_handshake_header
s = encode_ros_handshake_header(header)
File "/home/toru/catkin_ws/install/lib/python3/dist-packages/rosgraph/network.py", line 403, in encode_ros_handshake_header
fields = [k + b"=" + v for k, v in sorted(encoded_header.items())]
File "/home/toru/catkin_ws/install/lib/python3/dist-packages/rosgraph/network.py", line 403, in
fields = [k + b"=" + v for k, v in sorted(encoded_header.items())]
TypeError: can't concat dict to bytes
Inbound TCP/IP connection failed: can't concat dict to bytes

where encoder_header was:
{b'error': {'error': 'unhandled connection'}}
and this line of code was expecting encoder_header to be a dict[str: str].
Hence, the fix.
Please let me know if you need more info!

@peci1
Copy link
Contributor

peci1 commented Apr 13, 2023

Is there a way to reproduce the problem?

@g-gemignani
Copy link
Contributor Author

I tried to reproduce the issue but failed to do so. If you do not think that this fix is worth merging, feel free to decline it. Up to you :)

@peci1
Copy link
Contributor

peci1 commented Apr 13, 2023

I'm not a maintainer of rospy, so I'll leave that decision on @mjcarroll .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants