serrors: Add serrors package

[lukedirtwalker]

This should in the long term replace:
-errors.New
-common.NewBasicError

The package is ready to be used with Go1.13 (or xerrors) Is and As.

Also it provides all the functionality that was provided by common.BasicError/common.MultiError.

By putting it in a serrors package we can make the API slightly nicer to use.

Contributes #3042

---

This change is 

[oncilla]

Reviewed 2 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, 8 unresolved discussions (waiting on @lukedirtwalker and @oncilla)

---

go/lib/serrors/errors.go, line 27 at r1 (raw file):

// ErrorWrapper allows recursing into nested errrors.
type ErrorWrapper interface {

Wrapper

---

go/lib/serrors/errors.go, line 49 at r1 (raw file):

}

func (e basicError) Is(err error) bool {

Somewhere we should have a description what is Is-able and how.

It looks like basic error can be the same if they have the same errOrMsg.str, which is fine imo.
But an Example would be nice.

---

go/lib/serrors/errors.go, line 61 at r1 (raw file):

}

// TODO do we need self ASing? that would mean the type would need to be public.

Hm, not really imo

if we need to, we can add support in the future. Should be a backwards compatible change anyway.

---

go/lib/serrors/errors.go, line 153 at r1 (raw file):

}

// errList is the internal error interface implementation of MultiError.

s/MultiError/error List

---

go/lib/serrors/errors.go, line 160 at r1 (raw file):

}

// FmtError formats e for logging. It walks through all nested errors, putting each on a new line,

s/e/the error

---

go/lib/serrors/errors_test.go, line 70 at r1 (raw file):

		cause: &testToTempErr{msg: "timeout", timeout: true},
	})
	assert.False(t, serrors.IsTimeout(timeoutWrappingNoTimeout))

huh, why is this not true?

---

go/lib/serrors/errors_test.go, line 83 at r1 (raw file):

		cause: &testToTempErr{msg: "timeout", temporary: true},
	})
	assert.False(t, serrors.IsTemporary(tempWrappingNoTemp))

ditto

---

go/lib/serrors/errors_test.go, line 177 at r1 (raw file):

		assert.True(t, xerrors.Is(err1, err1))
		assert.True(t, xerrors.Is(err2, err2))
		assert.False(t, xerrors.Is(err1, err2))

This behavior should be mentioned somewhere, maybe ExampleNew?

[lukedirtwalker]

Reviewable status: 2 of 3 files reviewed, 8 unresolved discussions (waiting on @oncilla)

---

go/lib/serrors/errors.go, line 27 at r1 (raw file):

Previously, Oncilla wrote…

Wrapper

Done.

---

go/lib/serrors/errors.go, line 49 at r1 (raw file):

Previously, Oncilla wrote…

Somewhere we should have a description what is Is-able and how.

It looks like basic error can be the same if they have the same errOrMsg.str, which is fine imo.
But an Example would be nice.

That depends

err := serrors.New("foo")
wrap1 := serrors.WrapStr("wrap", err)
wrap2 := serrors.WrapStr("wrap", err)
xerrors.Is(wrap1, wrap2) == true

but

xerrors.Is(serrors.New("foo"), serrors.New("foo")) == false

I added some details to the package doc and to the methods

---

go/lib/serrors/errors.go, line 61 at r1 (raw file):

Previously, Oncilla wrote…

Hm, not really imo

if we need to, we can add support in the future. Should be a backwards compatible change anyway.

Done.

---

go/lib/serrors/errors.go, line 153 at r1 (raw file):

Previously, Oncilla wrote…

s/MultiError/error List

Done.

---

go/lib/serrors/errors.go, line 160 at r1 (raw file):

Previously, Oncilla wrote…

s/e/the error

Done.

---

go/lib/serrors/errors_test.go, line 70 at r1 (raw file):

Previously, Oncilla wrote…

huh, why is this not true?

it's a no timeout that wraps a timeout. The variable name was wrong, also I made it more explicit.

---

go/lib/serrors/errors_test.go, line 83 at r1 (raw file):

Previously, Oncilla wrote…

ditto

ditto

---

go/lib/serrors/errors_test.go, line 177 at r1 (raw file):

Previously, Oncilla wrote…

This behavior should be mentioned somewhere, maybe ExampleNew?

Done.

[oncilla]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

---

go/lib/serrors/errors_test.go, line 218 at r2 (raw file):

	// with same text are seen as the same thing:
	fmt.Println(xerrors.Is(err1, err2))

Also add a sample with:

wrap1 := serrors.WrapStr("wrap", err)
wrap2 := serrors.WrapStr("wrap", err)
xerrors.Is(wrap1, wrap2) == true

[oncilla]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

[oncilla]

Reviewable status: complete! all files reviewed, all discussions resolved

[lukedirtwalker]

Reviewable status: complete! all files reviewed, all discussions resolved

---

go/lib/serrors/errors_test.go, line 218 at r2 (raw file):

Previously, Oncilla wrote…

Also add a sample with:

wrap1 := serrors.WrapStr("wrap", err)
wrap2 := serrors.WrapStr("wrap", err)
xerrors.Is(wrap1, wrap2) == true

I don't think anyone should do that anyway so I'd rather not have an example. Also I don't think we should specify the behavior, so that no one relies on it.