Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use different error type for "internal" errors #8

Open
bodokaiser opened this issue Jul 21, 2014 · 3 comments
Open

use different error type for "internal" errors #8

bodokaiser opened this issue Jul 21, 2014 · 3 comments

Comments

@bodokaiser
Copy link
Contributor

Hello Roberto!

Have you thought of using a different error struct for internal errors (e.g. ErrUnsupported, ErrBadParameter, ErrUnknownTag)?

One use case where this may be handy is to decide if I send a http.StatusInternalServerError or a http.StatusBadRequest or maybe even do a panic because of wrong validation settings.

@bodokaiser
Copy link
Contributor Author

I suggest to change TextError to:

ValidationError - Used when value does not pass validation constraint.

type ValidationError string

func (err ValidationError) Error() string {
    return err
}

func (err ValidationError) MarshalText() ([]byte, error) {
    // ...
}

RuntimeError - Used constraints (tags) or values are wrong.

type RuntimeError string

func (err ValidationError) Error() string {
    return err
}

func (err ValidationError) MarshalText() ([]byte, error) {
    // ...
}

In a http.HandlerFunc we then could do:

func HandlerFunc(w http.ResponseWriter, r *http.Request) {
    // some error from validator
    var err error

    switch t := err.(type) {
    case validator.ValidationError:
        http.Error(w, err.Error(), http.StatusBadRequest)
    case validator.RuntimeError:
        http.Error(w, err.Error(), http.StatusInternalServerError)
    }
}

@bodokaiser
Copy link
Contributor Author

But @robteix must decide if this is a breaking change or not. I personally would consider it as one as people may do error checks agains ErrUnknownTag or something.

@rselbach
Copy link
Contributor

It is breaking but go ahead. Just do a PR to master and I'll release it as
v2. Thanks!

-rst

Sent from my mobile
Please forgive the spelling and brevity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants