On Fri, Nov 6, 2015 at 11:24 AM, Neil Conway <[email protected]> wrote:
> Hi Marco, > > On Fri, Nov 6, 2015 at 10:57 AM, Marco Massenzio <[email protected]> > wrote: > > So, in addition to what Michael suggests, and in line with Alex's > > exhortation to have "excellent core error reporting facilities," I would > > also like to propose that we consider adopting a pattern I've seen > > elsewhere: > > > > if (errorOccurred) { > > string msg = "Dude, something went wrong!"; > > LOG(ERROR) << msg; > > return Error(msg); > > } > > > > or a variation thereof. > > I'd like to understand what you see as the advantage of doing this *in > addition to* adding line number + source file information to the Error > object itself. i.e., if the Error carries around information about the > context in which it was constructed, this would seem to avoid the need > to grep for error strings. > I may be wrong here, but adding (filename, line_nr) to the Error() object is performance-impact and may need to be disabled in Production environments (again, not sure, this is certainly what we do with loggers like log4j, etc.). Having a trail of logs (even with, possibly, incomplete information) definitely helps in narrowing down the scope of diagnosis of issues in large clusters, especially when the folks doing the first-level debugging may not be necessarily familiar with the codebase. > My concern with the pattern you suggested is that (a) it adds logic at > every site where we're generating errors, there is no logic added - you still need to check for errors when returning the Error() object all I was suggesting to add was the LOG(ERROR); nothing else. > and (b) not every place > where we construct an Error will have enough context about what went > wrong to be able to report the problem accurately. > Absolutely - but it adds another "signal", especially when chasing not-easy-to-reproduce issues.
