Catch exceptions thrown in handler functions, send status to client

[achim-k]

Public-Facing Changes

None

Description

• Refactors the way handler functions are run in a separate thread. It's better to do that in a unified way in the server rather than doing that for ROS 1 & 2 differently as it was implemented in Run client handler functions in separate thread #165
• Catch exeptions thrown in handler functions and send status message to client

Fixes #148

[jtbandes]

This seems like a good change overall. I have seen C++ codebases benefit greatly from a Status type, e.g. https://abseil.io/docs/cpp/guides/status, so that would be a nice improvement/cleanup to consider. I'm not sure if there is one already in the ROS package ecosystem?

[achim-k]

This seems like a good change overall. I have seen C++ codebases benefit greatly from a Status type, e.g. https://abseil.io/docs/cpp/guides/status, so that would be a nice improvement/cleanup to consider. I'm not sure if there is one already in the ROS package ecosystem?

I wonder if we should not simply throw exceptions if something went wrong. We already use exceptions in other places, so there is no good reason to not use them. This seems to be also what the C++ core guidelines recommend: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e25-if-you-cant-throw-exceptions-simulate-raii-for-resource-management

[jtbandes]

Since we already have a detailed message as part of the error, I wonder if we really need a separate subclass for each type of error, or could you just use ServiceError for all of these?

[achim-k]

This is part of the public API, so not sure what error granularity is best here. I guess I will remove all error types that are not explicitly caught in the server code