Between RVO and move semantics we shouldn't be (deep) copying Status objects very much. That being said, while I agree that the Status pattern is not optimal in a lot of ways, I think changing it to be templated is going to be very painful (if not impossible due to public API concerns). Piping through an additional out-param seems like a prudent approach without boiling the ocean.
FWIW Rust does have a templated result type which is used pervasively in the standard library and elsewhere, and I do think it's a great pattern. It has one difference though - it templates on both a success type _and_ an error type, which avoids the need to have an out-param in almost all cases. I'd be in favor of moving to something like that, but it requires something like C++ variants to do correctly (boost might have this, not sure). I bring this up only because I think there is an opportunity to improve our error handling pattern a lot, but I don't think adding an error param is necessarily going far enough for the amount of effort it would require. - Dan On Mon, Apr 24, 2017 at 1:02 PM, David Alves <[email protected]> wrote: > I'd be pro having an _optional_ PB or some other optional way to keep extra > metadata in a status. > In most cases (no extra metadata) this wouldn't have an additional cost > when we're copying statuses around and the PB would have the obvious > advantage of being easy to send to clients, when we need more error > information to handle things properly. > I don't think extra args are going to be scalable since we'd need to > basically pipe them everywhere we could get an EIO. > Regarding the templated option, how would the caller know that it can be > downcast? Is it always in the same type of error? Would you need rtti? > > -david > > On Mon, Apr 24, 2017 at 12:46 PM, Alexey Serbin <[email protected]> > wrote: > > > Hi Andrew, > > > > The instances of Status objects are copied around very often, so putting > > something heap-allocated into the Status class would require dealing with > > that (wrapping those into shared_ptr? But I'm not sure it's worth it). > > > > As for down-casting: would it be possible in your case to limit the > > exchange of ExtendedStatus instances only to some particular methods and > > make sure your methods always return the ExternedStatus? That would > allow > > to avoid downcasting. > > > > I considered a similar approach while propagating RPC error codes along > > with Status in the code around connection negotiation, but eventually I > > decided to go with an additional output parameter for a couple of > methods. > > > > > > Best regards, > > > > Alexey > > > > > > On 4/24/17 12:33 PM, Andrew Wong wrote: > > > >> Hi everyone, > >> > >> In my work on disk failure handling, it would be very useful to pass > extra > >> information along with Statuses. To add some context, I would like to > >> propagate the uuid of a failed disk to the high-level operations that > may > >> have caused the failure (e.g. FlushDeltaMemStoresOp.Perform(), etc.), > >> which > >> can then error-handle based on this uuid (e.g. tell the FsManager, > >> DataDirManager, etc. that disk <uuid> is dead) instead of failing a > check. > >> > >> Since this is such a widely used class, this sort of change warrants > some > >> discussion, particularly if the functionality would be useful elsewhere. > >> > >> A couple possible implementations: > >> > >> 1. Have a template class that extends Status with a pointer to a > >> heap-allocated T. When handling errors, call a templatized function that > >> downcasts the Status to a StatusWithExtra<T> and returns the T. This is > >> moderately flexible, and wouldn't need to be changed as it gets used > more > >> often, but downcasting will warrant careful consideration in terms of > >> type-safety. > >> > >> 2. Have Status have a StatusExtrasPB message that has a number of > objects > >> within. As passing data through Status becomes used in other areas of > >> code, > >> the StatusExtrasPB can be extended. This would avoid downcasting, > >> extending > >> Status directly. > >> > >> These are just a couple of ideas, but I'd be interested in seeing what > >> people think and whether anyone had other ideas for this sort of > >> message-passing. > >> > >> > >> Andrew > >> > >> > > >
