Rename Error interface method to avoid collisions with gocql

api/graphite.go

@@ -251,7 +251,7 @@ func (s *Server) renderMetrics(ctx *middleware.Context, request models.GraphiteR
 	out, meta, err := s.executePlan(execCtx, ctx.OrgId, plan)
 	if err != nil {
 		err := response.WrapError(err)
-		if err.Code() != http.StatusBadRequest {
+		if err.HTTPStatusCode() != http.StatusBadRequest {
 			tracing.Failure(execSpan)
 		}
 		tracing.Error(execSpan, err)

api/response/error.go

@@ -4,13 +4,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"runtime/debug"
-
-	log "github.com/sirupsen/logrus"
 )
 
 type Error interface {
-	Code() int
+	//This could have just been `Code`, but that would make it accidentally share an interface with gocql and lead to nonsense HTTP codes
+	//See https://github.com/grafana/metrictank/issues/1678
+	HTTPStatusCode() int
 	Error() string
 }
 
@@ -21,18 +20,15 @@ type ErrorResp struct {
 
 func WrapError(e error) *ErrorResp {
 	if err, ok := e.(*ErrorResp); ok {
-		err.ValidateAndFixCode()
 		return err
 	}
 	resp := &ErrorResp{
 		err:  e.Error(),
 		code: http.StatusInternalServerError,
 	}
 	if _, ok := e.(Error); ok {
-		resp.code = e.(Error).Code()
+		resp.code = e.(Error).HTTPStatusCode()
 	}
-
-	resp.ValidateAndFixCode()
 	return resp
 }
 
@@ -56,10 +52,8 @@ func WrapErrorForTagDB(e error) *ErrorResp {
 	}
 
 	if _, ok := e.(Error); ok {
-		resp.code = e.(Error).Code()
+		resp.code = e.(Error).HTTPStatusCode()
 	}
-
-	resp.ValidateAndFixCode()
 	return resp
 }
 
@@ -81,6 +75,10 @@ func (r *ErrorResp) Error() string {
 	return r.err
 }
 
+func (r *ErrorResp) HTTPStatusCode() int {
+	return r.code
+}
+
 func (r *ErrorResp) Code() int {
 	return r.code
 }
@@ -98,13 +96,4 @@ func (r *ErrorResp) Headers() (headers map[string]string) {
 	return headers
 }
 
-func (r *ErrorResp) ValidateAndFixCode() {
-	// 599 is max HTTP status code
-	if r.code > 599 {
-		log.Warnf("Encountered invalid HTTP status code %d, printing stack", r.code)
-		debug.PrintStack()
-		r.code = http.StatusInternalServerError
-	}
-}
-
 var RequestCanceledErr = NewError(499, "request canceled")

cluster/node.go

@@ -143,7 +143,7 @@ func (r *Error) Error() string {
 }
 
 // implement response.Response
-func (r *Error) Code() int {
+func (r *Error) HTTPStatusCode() int {
 	return r.code
 }
 

errors/errors.go

@@ -15,7 +15,7 @@ func NewInternalf(format string, a ...interface{}) Internal {
 	return Internal(fmt.Sprintf(format, a...))
 }
 
-func (i Internal) Code() int {
+func (i Internal) HTTPStatusCode() int {
 	return http.StatusInternalServerError
 }
 
@@ -33,7 +33,7 @@ func NewBadRequestf(format string, a ...interface{}) BadRequest {
 	return BadRequest(fmt.Sprintf(format, a...))
 }
 
-func (b BadRequest) Code() int {
+func (b BadRequest) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 

expr/func_aspercent_test.go

@@ -22,7 +22,7 @@ func (e errAsPercentNumSeriesMismatch) Error() string {
 	return fmt.Sprintf("asPercent got %d input series but %d total series (should  be same amount or 1)", e.numIn, e.numTotal)
 }
 
-func (e errAsPercentNumSeriesMismatch) Code() int {
+func (e errAsPercentNumSeriesMismatch) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 

expr/parse.go

@@ -34,7 +34,7 @@ func (e ErrBadArgument) Error() string {
 	return fmt.Sprintf("argument bad type. expected %s - got %s", e.exp, e.got)
 }
 
-func (e ErrBadArgument) Code() int {
+func (e ErrBadArgument) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 
@@ -47,7 +47,7 @@ func (e ErrBadArgumentStr) Error() string {
 	return fmt.Sprintf("argument bad type. expected %s - got %s", e.exp, e.got)
 }
 
-func (e ErrBadArgumentStr) Code() int {
+func (e ErrBadArgumentStr) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 
@@ -57,7 +57,7 @@ func (e ErrUnknownFunction) Error() string {
 	return fmt.Sprintf("unknown function %q", string(e))
 }
 
-func (e ErrUnknownFunction) Code() int {
+func (e ErrUnknownFunction) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 
@@ -69,7 +69,7 @@ func (e ErrUnknownKwarg) Error() string {
 	return fmt.Sprintf("unknown keyword argument %q", e.key)
 }
 
-func (e ErrUnknownKwarg) Code() int {
+func (e ErrUnknownKwarg) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 
@@ -83,7 +83,7 @@ func (e ErrBadKwarg) Error() string {
 	return fmt.Sprintf("keyword argument %q bad type. expected %T - got %s", e.key, e.exp, e.got)
 }
 
-func (e ErrBadKwarg) Code() int {
+func (e ErrBadKwarg) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 
@@ -95,7 +95,7 @@ func (e ErrKwargSpecifiedTwice) Error() string {
 	return fmt.Sprintf("keyword argument %q specified twice", e.key)
 }
 
-func (e ErrKwargSpecifiedTwice) Code() int {
+func (e ErrKwargSpecifiedTwice) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }
 

expr/tagquery/expression.go

@@ -18,7 +18,7 @@ func (i InvalidExpressionError) Error() string {
 	return fmt.Sprintf("Invalid expression: %s", string(i))
 }
 
-func (i InvalidExpressionError) Code() int {
+func (i InvalidExpressionError) HTTPStatusCode() int {
 	return http.StatusBadRequest
 }