RSDK-2758 Add an exception system

[benjirewis]

RSDK-2758

Redo of #100. Copied much of the base code from @zaporter-work's original implementation, and the goals of this PR remain basically the same as the last one:

This provides a base ViamException class and a few derived classes. The purpose of these exceptions is threefold:
1. To provide a base to build on if we wish to pursue more advanced error creation in the future.
2. To help users easily understand if an error came from our code or not.
3. To facilitate error-catching and recovery behavior (ex: a DuplicateResourceException might be handled differently than a ResourceNotFoundException)

Replaces all instances of throw std::runtime_error, throw std::invalid_argument, throw std::length_error and throw "foo" with throws of the new exception classes. Note that none of the throw calls are touched in the examples directory, as those represent example client or module code that should not be using our internal exception system.

[benjirewis]

I need to add test_exception.cpp with some tests, but thought I'd gather feedback before continuing.

[stuqdog]

(I noticed this is in draft but figured I'd take a look. If I was over-eager, apologies! Please re-request review if there's anything still coming).

One or two minor comments but looks good! Note that I expect there will be some conflict between this PR and the refactor registry PR, so we should make sure that whichever one gets merged second properly converts over any lingering runtime_exceptions.

[stuqdog]

just a heads up that all of these "Unable to get service descriptor for {resourcetype} service" errors in {resourcetype}.cpp are being removed in the refactor-registry PR so some rebase conflict will be happening!

[acmorrow]

So, when this came up before (per @zaporter-work), and again now, it sends me down a deep rabbit hole of studying the gRPC error model and error_details and C++ system_error subsystem and thinking about std::error_condition and std::error_category, and std::expected too, and how it should all fit together. And then I stall out because I'm worried it is all too complex and we shouldn't bother.

Do we actually need sub-classes of ViamException? Or should there just be one which carries a std::error_condition with it? Should we create a std::error_category for gRPC errors? For ours? Could we use error_details to transmit category information from server to client? That might allow the client side to actually throw the server side error with fidelity. Could generic components and services have their own error spaces?

Getting the error handling model for the SDK right is very important, because once one is adopted it is more or less impossible to change it later. On the other hand, our current informal setup is less than ideal, and maybe we shouldn't let the great be the enemy of the good, etc.

[stuqdog]

So, when this came up before (per @zaporter-work), and again now, it sends me down a deep rabbit hole of studying the gRPC error model and error_details and C++ system_error subsystem and thinking about std::error_condition and std::error_category, and std::expected too, and how it should all fit together. And then I stall out because I'm worried it is all too complex and we shouldn't bother.

Do we actually need sub-classes of ViamException? Or should there just be one which carries a std::error_condition with it? Should we create a std::error_category for gRPC errors? For ours? Could we use error_details to transmit category information from server to client? That might allow the client side to actually throw the server side error with fidelity. Could generic components and services have their own error spaces?

Getting the error handling model for the SDK right is very important, because once one is adopted it is more or less impossible to change it later. On the other hand, our current informal setup is less than ideal, and maybe we shouldn't let the great be the enemy of the good, etc.

I am definitely not an expert on this and a lot of your concerns hadn't even occurred to me so happy to be corrected, but: it seems to me we can accomplish pretty much everything we have here by carrying forward the std::error_condition and having an enum of the common error cases to match on for when we care about what kind of error we received. At a minimum it offers extensibility and specificity without creating child class proliferation.

[benjirewis]

I switched to a combination of std::error_code and std::error_category and got rid of the proliferation of child classes. Let me know what you think! I didn't really know what I was doing and used this article for reference.

[acmorrow]

I like where this is headed. My biggest questions are around whether this should all be in terms of error_condition rather than error_code, and whether we can do more with gRPC errors. I suspect two of my comments on that front are somewhat in tension, but that's mostly because I'm not yet 100% sure what a good solution should look like or how it should behave.

[acmorrow]

I like this, I think. I want to contemplate a bit how the gRPC error code and GRPCException thing should work w.r.t. to grpc::Status a bit more. Everything else here looks good though.

Probably time to promote it from draft because I expect this will merge in a form close to what is here now.

[benjirewis]

Probably time to promote it from draft because I expect this will merge in a form close to what is here now.

SGTM; I'll take some time to write tests in the meantime.

[acmorrow]

LGTM. We have other uses of grpc in our API currently so I'm not too concerned about one more.

[stuqdog]

Sorry for review delay, lgtm!