ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
753 stars 910 forks source link

Subscriber traps exceptions in callbacks #1673

Open jellevos opened 5 years ago

jellevos commented 5 years ago

I have spent some time looking up how exceptions are handled in ROS and I am bit confused. The following forum post makes me believe that you could try-except around spin() to catch exceptions and restart a loop: https://answers.ros.org/question/274798/how-are-exceptions-dealt-with-in-callback/

I do not see this behaviour when it comes to Subscribers. When I dug into the Subscriber's code I found this: https://github.com/ros/ros_comm/blob/c3d7c393fee534b7bbecbb495fba706196a06bcf/clients/rospy/src/rospy/topics.py#L736

The _invoke_callback function traps exceptions. I was wondering if this is not conflicting to how exceptions are normally expected to bubble up through a system. Is this the intended behaviour? And would there be problems if I were to extend the Subscriber and overwrite this behaviour, in order to catch exceptions somewhere higher up?

cwecht commented 5 years ago

Well, at least this is an inconsistency between rospy and roscpp. But we can not changes this without breaking existing code, since somebody might rely on this behavior. Not trapping behavior could be opt-in; I will have a look at this.

cwecht commented 5 years ago

It turns out, that roscpp is inconsistent within it self. If an exception in a timer-callback is thrown, the timer will stop to work, because after the callback, things need to be done to set up the next invocation of the callback, which are not done, if an exception is thrown. In a service-call, the exception is trapped.

For the moment I would discourage to throw exceptions in callbacks in general, as there is no consistent behavior in roscpp. To keep it simple and consistent, I would recommend, to apply this rule even for rospy.