Wrap errors

[dshemin]

I think will be better to use go 1.13 errors wrapping.

[hnakamur]

Hi, thanks for your pull request.

However I think wrapping an error should be used where it is absolutely necessary to handle the error by checking the internal error type or details. I became to think so after reading the following.

Working with Errors in Go 1.13 - The Go Programming Language

In other words, wrapping an error makes that error part of your API. If you don’t want to commit to supporting that error as part of your API in the future, you shouldn’t wrap the error.

In this library, I think there is no such place since it does not return public error types. So I think it is better not to wrap errors.

[dshemin]

Yes, it's true, you should do something when it should be done. But this is a library and you can't predict how it will be used. For instance, I use it in my project and I have special cases for context.Canceled and context.DeadlineExceeded and I don't want to use string comparison for errors in my production code. I prefer to use errors.Is and errors.As, for instance like this if errors.Is(err, context.Canceled) { ... }. So I had to write next code, to adopt errors from go-scp package:

func adoptError(err error) error {
    if err == nil {
        return nil
    }

    switch {
    case strings.Contains(err.Error(), context.Canceled.Error()):
        err = wrapError{
            msg: err.Error(),
            err: err,
        }
    case strings.Contains(err.Error(), context.DeadlineExceeded.Error()):
        err = wrapError{
            msg: err.Error(),
            err: err,
        }
    }
    return err
}

type wrapError struct {
    msg string
    err error
}

func (e wrapError) Error() string { return e.msg }
func (e wrapError) Unwrap() error { return e.err }

[hnakamur]

Thanks for your explanation. I'm convinced now, so I merge this. Thanks!