On 6 February 2014 22:00, Nate Finch <nate.fi...@canonical.com> wrote:
> So, I took a deeper dive into Tim's newly renamed arrar package, and
> compared it to Roger's errgo package.
>
> Before I go into the implementations, I want to write down some goals:
>
> 1: Make it easy to wrap errors from deeper in the code with context from the
> local scope.
> 2: Mask errors that are merely implementation details from callers.
> 3: Give a stack trace of the root error for debugging purposes.
>
> The packages are very similar in what they're trying to do - maintain an
> ordered list of errors that represent one logical error to the end user, and
> make it easy to wrap an existing error in one of their special errors,
> without losing information about the wrapped error.
>
> I think both of them need examples to show how they're intended to be used,
> but once I played around with them in actual code, they made sense. I also
> think both of them could use with some name tweaking to make the intent of
> the code a little more obvious.
>
> I like that Roger's package has a concrete type that can be reused, and
> interfaces you can implement if you don't want to use the base type. I would
> tend to do the former a lot, something like
>
> type NoConfigurationError errgo.Err
>
> However, the concept of a Diagnosis seems overly complicated to me. I
> pretty much never want to encourage people to look at the underlying error,
> and diagnosis seems to want to do that in some cases.
To clarify a little:
errgo.Diagnosis(err) is almost exactly equivalent
to arrar.GetErrorStack(err)[0], and both are
a necessary and important way to access the
underlying error when it contains information important
to the caller. Custom error types in Go can contain
useful info and are an important part of the language.
I don't really see there's a significant difference between
if errgo.Diagnosis(err) == io.EOF {
and
if arrar.Check(err, func(err error) bool {return err == io.EOF}) {
The whole point of classifying errors is that we
need to know something about the error in question - in
arrar it's the last error that wasn't an annotation; in
errgo, the concept is made more explicit.
> I think what I really want is Roger's errgo package, but with the Diagnosis
> code removed, and just expose underlying errors through the Cause() method.
As an example of the motivation for the Diagnosis concept,
and the kind of thing I'm wanting to guard against,
here's a hypothetical example. We want to make a network
connection, but need to read a configuration file first:
http://play.golang.org/p/zXqLB9sb_e
Consider some of the possible errors from ReadPacketFromNetwork,
(each preceded by the error that will be used if we call arrar.Check
on the error):
*os.PathError cannot read configuration file: open
/home/.conf: no such file or directory
io.ErrUnexpectedEOF cannot read configuration file: cannot read
address: unexpected EOF
*net.OpError cannot dial "foo.com": dial tcp
12.34.56.78 connection timed out
io.ErrUnexpectedEOF network read failed: unexpected EOF
Note that io.ErrUnexpectedEOF can be returned as the result
of two very different operations. Suppose we have a package
that's using the above code, and we observe that sometimes
a server is faulty and delivers truncated data. We might well
implement something like this:
http://play.golang.org/p/NNqryXwz95
But that would be faulty code, in a very subtle and easy to
overlook way, because it will do the wrong thing if the configuration
file happens to be truncated.
By contrast, here's how the two pieces of code would look using errgo:
http://play.golang.org/p/uaTjyO7_se
http://play.golang.org/p/Ze_YTCcfzm
Note that, even ignoring the doc comment, it is easy to tell by
looking at the high
level function, ReadPacketFromNetwork, what classifiable errors might
be returned from
it. This makes it easy to write the doc comment - the contract with the
caller is clear.
This, then, is the essence of the distinction between Diagnosis and
Cause - the Diagnosis is part of the contract of a function - it is
something that a caller may depend on and that it is the responsibility
of a function to maintain. The Cause is just some arbitrary underlying
error - there are many possible error causes, but we don't want arbitrary
calling code to become dependent on them, because every error cause
reflects some implementation detail, and depending on implementation
details makes the code base more brittle and harder to change.
William correctly pointed out that this does impose a burden on us -
if we do want to allow another kind of error diagnosis to be returned
from a function, we are obliged to change all the intermediate paths to
pass it through. My hope is that this will not actually prove much of a
problem in practice, and that it will be much outweighed by being able
to *know* that no caller can be depending on the unmentioned diagnoses
and the resulting freedom to change code.
We currently have some extremely subtle dependencies between distantly
related pieces of code, and I would really like to make those dependencies
evident.
This doesn't mean that we should do all of that immediately,
but it's a direction that I think would be very healthy for the
code base overall.
> I'd like to then steal arrar's function for returning a list of the
> underlying errors (GetErrorStack), and I like his string formatting better
> for the stack-trace-lite (DetailedErrorStack), but I wouldn't bother with
> formatting parameter, because I honestly will never care.
I deliberately made the error info formatting fit on a single line
so it would work well in log files, but it would be easy to implement
an alternative format too.
> However, I also was an honest-to-god stacktrace available from the root
> error, because both implementations can miss important information about how
> you got to the root error that a real stacktrace wouldn't miss.
I'd be interested to see an example of when you think the errgo/arrar
approach would not produce sufficient information (assuming that
errors are Traced/Wrapped by default). The honest-to-god stack trace
is unfortunately insufficient in a concurrent world where errors may be
(and often are) transferred between goroutines.
cheers,
rog.
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
