gramaziokohler / roslibpy

Python ROS Bridge library
MIT License
276 stars 57 forks source link

Remove usage of `raise Exception` in favor of more specific exception types #105

Closed DomenicP closed 1 year ago

DomenicP commented 1 year ago

Currently roslibpy has a number of places where Exception is directly raised instead of a more specific error type. This means that users of the library have to use except: or except Exception: when calling methods that may raise an exception, which is not generally regarded to be a good Python practice. For example,

ros = roslibpy.Ros(host=..., port=...)

...

try:
    ros.run()
except Exception:  # pylint: disable=broad-except
    # roslibpy will attempt to connect immediately, but the robot may not be
    # present. This is fine, we can just ignore it.
    pass

Note the pylint disable message required for the broad-except violation.

One easy way to fix this is to raise more specific error types instead of raising Exception directly. This PR goes through and eliminates all direct occurrences of raise Exception from the library code (there are still a few in the tests). In a lot of these cases the exception was being raised due to a timeout so switching to TimeoutError seemed reasonable. I also introduced the ServiceException class to be raised when a blocking service call fails.

This change should be backwards compatible since any code that was catching Exception should also catch subclasses of Exception.

Open to discussion or suggestions around this PR since there are a number of ways to approach this topic.

What type of change is this?

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

DomenicP commented 1 year ago

@gonzalocasas Thanks for the prompt response! I was wondering if you had any timeline or schedule for publishing the next release of this library? I'd be glad to help with publishing a minor release if needed.

gonzalocasas commented 1 year ago

@DomenicP Done! 1.4.0 is already on pypi, and the build on conda-forge will be auto-triggered soon