in-toto-golang
in-toto-golang copied to clipboard
Error handling
Please fill in the fields below to submit an issue or feature request. The more information that is provided, the better.
Description of issue or feature request:
With Go 13, Go got better error handling with functions like errors.Is()
or errors.As()
. We might want to adapt our code, so developers can use this functions with our library.
Current behavior:
Right now we are excessively using fmt.Errorf("..")
for error messages. Instead we should declare global Error types like in the Symlink implementation and return error types:
// ErrSymCycle signals a detected symlink cycle in our RecordArtifacts() function.
var ErrSymCycle = errors.New("symlink cycle detected")
This way we have error documentation and can use errors.Is(err, ErrSymCycle)
for error checking. This is a huge benefit over string checking with if err.Error() == "..." { .. }
, because with the latter we can run into a nil pointer dereference if Error() returns nil.
Expected behavior:
Use documented error types like:
// ErrSymCycle signals a detected symlink cycle in our RecordArtifacts() function.
var ErrSymCycle = errors.New("symlink cycle detected")
More about new Go 1.13 error handling can be found here: https://blog.golang.org/go1.13-errors
Feedback @SantiagoTorres @lukpueh ?
This sounds like a better, more idomatic way to do error handling. It also sounds it'd be super helpful to improve testing.
Is there a way to do this gradually?
@SantiagoTorres Yes, I just pushed 3 new error types in my latest in-toto-run commit. The error handling gets so much better with this.
We get basically from something like this:
expectedError := "Could not find a public key PEM block"
err = key.LoadRSAPublicKey("demo.layout.template")
if err == nil || !strings.Contains(err.Error(), expectedError) {
t.Errorf("LoadRSAPublicKey returned (%s), expected '%s' error", err,
expectedError)
}
to:
err = key.LoadRSAPublicKey("demo.layout.template")
if !errors.Is(err, ErrNoPEMBlock) {
t.Errorf("LoadRSAPublicKey returned (%s), expected '%s' error", err,
ErrNoPEMBlock.Error())
}
Cross-referencing with #5.
@shibumi is this issue still up ?