benchttp/engine

Implement proper error response for server endpoints

Closed this issue · 4 comments

Description

Tasks

Suggestions

Dependencies

  • Blocked by #12

Notes

Also: we need to define how to handle non-critical errors (the client sends invalid data, it needs to send it again correctly shaped) vs critical errors (aka status 500 errors).

Notably is there a reconnection procedure?

Types of error a run can produce:

  • ErrUnknownField or ErrUnknownPredicate when parsing the config json/yaml
  • InvalidConfigError: the config is invalid, it holds an array of errors (currently produces a pre-formatted string)
  • native error on config.Request.Value(): we cannot create the request for any reason
  • ErrConnection: when the recorder fails to connect to the url requested
  • NO error on context.DeadlineExceeded: we simply stop the run where we are and continue on with the metrics and the tests
  • ErrCanceled: when the run is explicitly canceled
  • native error on internal errors

Types of error the server can produce:

  • all of the above are forwarded
  • internal error on io.ReadAll(r.Body) error
  • internal error on configparse.JSON(body) error
  • internal error on encoding any response (report or progress)

Goal:

  • flag client errors differently than internal errors
  • provide maximum context on client errors
  • handle ErrCanceled differently than other errors
  • handle server and runner errors differently (?)
  • settle on if how we deal with context.DeadlineExceeded currently is what we want (ie: should going above the global timeout be an error ?)
graph TD;
  A(start)-->B{parse input config}
  B-->|invalid|BB[ParseConfigError: ErrUnknownField, ErrUnknownPredicate, error]
  B-->C{validate config}
  C-->|invalid|CC[InvalidConfigError]
  C-->D{create request}
  D-->|invalid|DD[ErrInternal, InvalidConfigError]
  D-->E{record requests}
  E-->|dead ping|EE[ErrConnection]
  E-->|canceled|EEE[ErrCanceled]
  E-->|unexpected|EEEE[ErrInternal]
  E-->F{respond}
  F-->|unexpected|FF[ErrInternal]
  F-->G(end)
Loading

Improvements can be found but the issue is viewed as implemented for now.