Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

improve error handling in query pipeline to set proper http codes #1462

Closed
Dieterbe opened this issue Sep 18, 2019 · 1 comment · Fixed by #1520
Closed

improve error handling in query pipeline to set proper http codes #1462

Dieterbe opened this issue Sep 18, 2019 · 1 comment · Fixed by #1520
Assignees
Milestone

Comments

@Dieterbe
Copy link
Contributor

Dieterbe commented Sep 18, 2019

e.g. FuncDivideSeries.Exec does this:

        if len(divisors) != 1 {
                return nil, errors.New(fmt.Sprintf("need 1 divisor series, not %d", len(divisors)))
        }

which result in http 500, but should be http 400

@Dieterbe Dieterbe added this to the sprint-2 milestone Oct 7, 2019
@fkaleo fkaleo modified the milestones: sprint-2, sprint-3 Oct 28, 2019
@Dieterbe Dieterbe changed the title some bad queries result in http 500 improve error handling in query pipeline to set proper http codes Oct 28, 2019
@replay replay self-assigned this Oct 28, 2019
@Dieterbe Dieterbe assigned Dieterbe and unassigned replay Nov 4, 2019
@woodsaj
Copy link
Member

woodsaj commented Nov 4, 2019

we already have an 'Error' interface defined in the api/response package.
https://github.com/grafana/metrictank/blob/master/api/response/error.go#L11-L14

If you have the expr package return errors that implement this interface, then they will be returned with the correct status code.

eg.
The errors in expr/parse.go: https://github.com/grafana/metrictank/blob/master/expr/parse.go#L16-L24

can just use response.NewError(http.StatusBadRequest, "argument missing") instead of errors.New("argument missing")

The other errors in this file that are using their own structs, just need to have a "Code() int" function added so they implement the interface.

eg

type ErrBadArgument struct {
	exp reflect.Type
	got reflect.Type
}

func (e ErrBadArgument) Error() string {
	return fmt.Sprintf("argument bad type. expected %s - got %s", e.exp, e.got)
}

func (e ErrBadArgument) Code() int {
  return http.StatusBadRequest
}

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

Successfully merging a pull request may close this issue.

4 participants