Use std::exception_ptr to transmit and store exceptions #1768
Conversation
| _callback(callback), | ||
| _session(session) | ||
| { | ||
| _ex = ex.ice_clone(); | ||
| _ex = ex; |
There was a problem hiding this comment.
should we call std::move(ex) here?
There was a problem hiding this comment.
It's not clear to me that we should move exception_ptr. Are they even movable?
According to https://en.cppreference.com/w/cpp/error/exception_ptr, exception_ptr meets the requirements of NullablePointer, which does not include movable.
| { | ||
| lock_guard lock(_mutex); | ||
| _exception = ex.ice_clone(); | ||
| _exception = ex; |
| { | ||
| request->exception(ex); | ||
| } | ||
| request->exception(e); |
| { | ||
| _childObserver.failed(ex.ice_id()); | ||
| _childObserver.failed(getExceptionId(ex)); |
There was a problem hiding this comment.
It's not clear to me what you want to move.
There was a problem hiding this comment.
The exception, but I think it's probably not worth trying to optimize these.
| } | ||
| catch (...) | ||
| { | ||
| // no closing reason |
There was a problem hiding this comment.
Would it be a bug to get this far? Should we just let it crash instead.
There was a problem hiding this comment.
I just translated the old code. With the old code, we don't set _closingReason unless the exception is one of the types we check.
| } | ||
| catch(const Ice::Exception&) | ||
| catch (...) |
There was a problem hiding this comment.
I thought we preferred catching std::exception as the most base exception? Everything should be deriving from it.
There was a problem hiding this comment.
That's correct, we assume all exceptions derive from std::exception.
However, in this particular code, we don't care: the fallback/catchall is to forward the exception. I see this as similar to:
catch { ... }
vs
catch (Exception) { ... }
in C#.
catch { } in C# corresponds to catch (...) in C++.
This PR updates the Ice C++ source code to use exception_ptr to transmit and store exceptions.
It's consistent with our async callback API, where errors are reported via a
std::function<void(exception_ptr)>.