PROPOSAL: Add support for domain errors

[jcchavezs]

As of now, when wrapping an error we keep the cause of the error meaning that if we want to handle the error in a upper layer we need to know details about the lower layers. For example:

package users

var ErrUserDoesNotExist = errors.New("user does not exist")

type Repository interface {
    GetUserById(id string) (*users.User, error)
}

package mysql

type mysqlRepository struct {
    ds datastore
}

func (mr *mysqlRepository) GetUserById (id string) (*users.User, error) {
    record, err := ds.exec(fmt.Sprintf("SELECT * FROM `users` WHERE id = %s", id))
    if err != nil {
        return nil, errors.Wrap(err, "could not retrieve user from persistence")
       // another option is to return ErrUserDoesNotExist
    }
}

Meaning that if I want to deal with the error, the handler should do something like:

package handlers

func (h *handler) UserProfile (c web.C, w http.ResponseWriter, r *http.Request) {
    uid := c.URLParams["uid"]
    u, err := h.userRepository.GetUserById(uid)

Option 1:
The handler should now about the implementation details of the database.

    if err.Cause() == mysql.NoRecordsFound {
        w.WriteHeader(http.StatusNotFound)
   }

Option 2:
We do a string comparation

    if err.Error() == "could not retrieve user from persistence"{
        w.WriteHeader(http.StatusNotFound)
   }

IMO this could be solved with another function:

func Map(causeErr error, meaningErr error) error {
	if causeErr == nil {
		return nil
	}
	err := &withDecoration{
		cause: causeErr,
		meaning: meaningErr,
	}
	return &withStack{
		err,
		callers(),
	}
}

Option 3:
We do a string comparation

    if err.Meaning() == ErrUserDoesNotExist {
        w.WriteHeader(http.StatusNotFound)
   }

I could open a PR for this.

[davecheney]

Use Cause, that's why it's there.

…

On Wed, 30 Aug 2017, 18:00 José Carlos Chávez ***@***.***> wrote: As of now, when wrapping an error we keep the cause of the error meaning that if we want to handle the error in a upper layer we need to know details about the lower layers. For example: package users var ErrUserDoesNotExist = errors.New("user does not exist") type Repository interface { GetUserById(id string) (*users.User, error) } package mysql type mysqlRepository struct { ds datastore } func (mr *mysqlRepository) GetUserById (id string) (*users.User, error) { record, err := ds.exec(fmt.Sprintf("SELECT * FROM `users` WHERE id = %s", id)) if err != nil { return nil, errors.Wrap(err, "could not retrieve user from persistence") // another option is to return ErrUserDoesNotExist } } Meaning that if I want to deal with the error, the handler should do something like: package handlers func (h *handler) UserProfile (c web.C, w http.ResponseWriter, r *http.Request) { uid := c.URLParams["uid"] u, err := h.userRepository.GetUserById(uid) *Option 1:* The handler should now about the implementation details of the database. if err.Cause() == mysql.NoRecordsFound { w.WriteHeader(http.StatusNotFound) } *Option 2:* We do a string comparation if err.Error() == "could not retrieve user from persistence"{ w.WriteHeader(http.StatusNotFound) } IMO this could be solved with another function: func Map(causeErr error, meaningErr error) error { if causeErr == nil { return nil } err := &withDecoration{ cause: causeErr, meaning: meaningErr, } return &withStack{ err, callers(), } } *Option 3:* We do a string comparation if err.Meaning() == ErrUserDoesNotExist { w.WriteHeader(http.StatusNotFound) } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#130>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcA-2PLmsoVNPeyMil1xNOOEADdcuLks5sdRaZgaJpZM4PHAba> .

[kristofferingemansson]

Cause does not fully solve the problem.
Let's say you do a json.Unmarshal in a function and get an error.
You want to indicate to the caller that the input data is wrong (so that somewhere higher up in the call chain some code might inform a user with e.g. http 400). This should be distinguishable from other errors like "database is down = 503, resource already exists = 409, and so on.
Currently with pkg/errors you have to chose if you want to return an error with a package/app-defined meaning, or the actual error for e.g. logging (the error returned by json.Unmarshal).

Side note: Also the way Wrap works by prepending a message to the previous error's message, one is unable to easily hide internal information from a user.
An authorization endpoint should print message invalid username or password and not invalid username or password: could not find user: sql: no rows in result set

[lhchavez]

I agree with the spirit of https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully 's "Assert errors for behaviour, not type" section. but in practice, creating an interface for each category of errors is a bit too onerous :( I found a comfortable middle ground that's more or less in the spirit of @jcchavezs' proposal in the sense that you can still get the domain error (I have named it Category inspired by C++), and it does provide Cause: https://github.com/omegaup/go-base/blob/master/errors.go

would that be something that could find its way into this repository?

[davecheney]

With the go team developing their own errors package, it’s unlikely this feature would be added to this package. You can just use omegaups version, they are already building on this package.

…

On 17 Dec 2018, at 12:05, lhchavez ***@***.***> wrote: I agree with the spirit of https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully 's "Assert errors for behaviour, not type" section. but in practice, creating an interface for each category of errors is a bit too onerous :( I found a comfortable middle ground that's more or less in the spirit of @jcchavezs' proposal in the sense that you can still get the domain error (I have named it Category inspired by C++), and it does provide Cause: https://github.com/omegaup/go-base/blob/master/errors.go would that be something that could find its way into this repository? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[knz]

FYI we have built domain errors in https://github.com/cockroachdb/errors