This reverts the changes from #4845, unfortunately changing the conte…

…nt type from plain text to JSON is a breaking change of the API and we should wait to do this at a major release.

Signed-off-by: Ed Welch <edward.welch@grafana.com>

docs/sources/upgrading/_index.md

@@ -84,22 +84,6 @@ Meanwhile, the legacy format is a string in the following format:
    [ ANNOTATIONS <label set> ]
 ```
 
-#### Error responses from API
-
-The body of HTTP error responses from API endpoints changed from plain text to
-JSON. The `Content-Type` header was previously already set incorrectly to
-`application/json`. Therefore returning JSON fixes this inconsistency.
-
-The response body has the following schema:
-
-```json
-{
-  "code": <http status code>,
-  "message": "<error message>",
-  "status": "error"
-}
-```
-
 #### Changes to default configuration values
 
 * `parallelise_shardable_queries` under the `query_range` config now defaults to `true`.

pkg/distributor/http.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/grafana/loki/pkg/loghttp/push"
 	util_log "github.com/grafana/loki/pkg/util/log"
-	serverutil "github.com/grafana/loki/pkg/util/server"
 	"github.com/grafana/loki/pkg/validation"
 )
 
@@ -30,7 +29,7 @@ func (d *Distributor) PushHandler(w http.ResponseWriter, r *http.Request) {
 				"err", err,
 			)
 		}
-		serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
 
@@ -66,7 +65,7 @@ func (d *Distributor) PushHandler(w http.ResponseWriter, r *http.Request) {
 				"err", body,
 			)
 		}
-		serverutil.JSONError(w, int(resp.Code), body)
+		http.Error(w, body, int(resp.Code))
 	} else {
 		if d.tenantConfigs.LogPushRequest(userID) {
 			level.Debug(logger).Log(
@@ -75,7 +74,7 @@ func (d *Distributor) PushHandler(w http.ResponseWriter, r *http.Request) {
 				"err", err.Error(),
 			)
 		}
-		serverutil.JSONError(w, http.StatusInternalServerError, err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}
 }
 

pkg/loki/loki.go

@@ -343,7 +343,6 @@ func (t *Loki) Run(opts RunOpts) error {
 
 	t.serviceMap = serviceMap
 	t.Server.HTTP.Path("/services").Methods("GET").Handler(http.HandlerFunc(t.servicesHandler))
-	t.Server.HTTP.NotFoundHandler = http.HandlerFunc(serverutil.NotFoundHandler)
 
 	// get all services, create service manager and tell it to start
 	var servs []services.Service

pkg/lokifrontend/frontend/transport/handler.go

@@ -17,13 +17,14 @@ import (
 	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
+	"github.com/weaveworks/common/httpgrpc"
+	"github.com/weaveworks/common/httpgrpc/server"
 
 	"github.com/grafana/dskit/tenant"
 
 	querier_stats "github.com/grafana/loki/pkg/querier/stats"
 	"github.com/grafana/loki/pkg/util"
 	util_log "github.com/grafana/loki/pkg/util/log"
-	serverutil "github.com/grafana/loki/pkg/util/server"
 )
 
 const (
@@ -32,6 +33,12 @@ const (
 	ServiceTimingHeaderName   = "Server-Timing"
 )
 
+var (
+	errCanceled              = httpgrpc.Errorf(StatusClientClosedRequest, context.Canceled.Error())
+	errDeadlineExceeded      = httpgrpc.Errorf(http.StatusGatewayTimeout, context.DeadlineExceeded.Error())
+	errRequestEntityTooLarge = httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "http: request body too large")
+)
+
 // Config for a Handler.
 type HandlerConfig struct {
 	LogQueriesLongerThan time.Duration `yaml:"log_queries_longer_than"`
@@ -220,11 +227,17 @@ func formatQueryString(queryString url.Values) (fields []interface{}) {
 }
 
 func writeError(w http.ResponseWriter, err error) {
-	if util.IsRequestBodyTooLarge(err) {
-		serverutil.JSONError(w, http.StatusRequestEntityTooLarge, "http: request body too large")
-		return
+	switch err {
+	case context.Canceled:
+		err = errCanceled
+	case context.DeadlineExceeded:
+		err = errDeadlineExceeded
+	default:
+		if util.IsRequestBodyTooLarge(err) {
+			err = errRequestEntityTooLarge
+		}
 	}
-	serverutil.WriteError(err, w)
+	server.WriteError(w, err)
 }
 
 func writeServiceTimingHeader(queryResponseTime time.Duration, headers http.Header, stats *querier_stats.Stats) {

pkg/storage/stores/shipper/compactor/deletion/request_handler.go

@@ -2,6 +2,7 @@ package deletion
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"time"
 
@@ -14,7 +15,6 @@ import (
 
 	"github.com/grafana/loki/pkg/util"
 	util_log "github.com/grafana/loki/pkg/util/log"
-	serverutil "github.com/grafana/loki/pkg/util/server"
 )
 
 // DeleteRequestHandler provides handlers for delete requests
@@ -40,21 +40,21 @@ func (dm *DeleteRequestHandler) AddDeleteRequestHandler(w http.ResponseWriter, r
 	ctx := r.Context()
 	userID, err := tenant.TenantID(ctx)
 	if err != nil {
-		serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
 
 	params := r.URL.Query()
 	match := params["match[]"]
 	if len(match) == 0 {
-		serverutil.JSONError(w, http.StatusBadRequest, "selectors not set")
+		http.Error(w, "selectors not set", http.StatusBadRequest)
 		return
 	}
 
 	for i := range match {
 		_, err := parser.ParseMetricSelector(match[i])
 		if err != nil {
-			serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 	}
@@ -64,7 +64,7 @@ func (dm *DeleteRequestHandler) AddDeleteRequestHandler(w http.ResponseWriter, r
 	if startParam != "" {
 		startTime, err = util.ParseTime(startParam)
 		if err != nil {
-			serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 	}
@@ -75,12 +75,12 @@ func (dm *DeleteRequestHandler) AddDeleteRequestHandler(w http.ResponseWriter, r
 	if endParam != "" {
 		endTime, err = util.ParseTime(endParam)
 		if err != nil {
-			serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 
 		if endTime > int64(model.Now()) {
-			serverutil.JSONError(w, http.StatusBadRequest, "deletes in future not allowed")
+			http.Error(w, "deletes in future not allowed", http.StatusBadRequest)
 			return
 		}
 	}
@@ -92,7 +92,7 @@ func (dm *DeleteRequestHandler) AddDeleteRequestHandler(w http.ResponseWriter, r
 
 	if err := dm.deleteRequestsStore.AddDeleteRequest(ctx, userID, model.Time(startTime), model.Time(endTime), match); err != nil {
 		level.Error(util_log.Logger).Log("msg", "error adding delete request to the store", "err", err)
-		serverutil.JSONError(w, http.StatusInternalServerError, err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -105,20 +105,20 @@ func (dm *DeleteRequestHandler) GetAllDeleteRequestsHandler(w http.ResponseWrite
 	ctx := r.Context()
 	userID, err := tenant.TenantID(ctx)
 	if err != nil {
-		serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
 
 	deleteRequests, err := dm.deleteRequestsStore.GetAllDeleteRequestsForUser(ctx, userID)
 	if err != nil {
 		level.Error(util_log.Logger).Log("msg", "error getting delete requests from the store", "err", err)
-		serverutil.JSONError(w, http.StatusInternalServerError, err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	if err := json.NewEncoder(w).Encode(deleteRequests); err != nil {
 		level.Error(util_log.Logger).Log("msg", "error marshalling response", "err", err)
-		serverutil.JSONError(w, http.StatusInternalServerError, "error marshalling response: %v", err)
+		http.Error(w, fmt.Sprintf("Error marshalling response: %v", err), http.StatusInternalServerError)
 	}
 }
 
@@ -127,7 +127,7 @@ func (dm *DeleteRequestHandler) CancelDeleteRequestHandler(w http.ResponseWriter
 	ctx := r.Context()
 	userID, err := tenant.TenantID(ctx)
 	if err != nil {
-		serverutil.JSONError(w, http.StatusBadRequest, err.Error())
+		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
 
@@ -137,28 +137,28 @@ func (dm *DeleteRequestHandler) CancelDeleteRequestHandler(w http.ResponseWriter
 	deleteRequest, err := dm.deleteRequestsStore.GetDeleteRequest(ctx, userID, requestID)
 	if err != nil {
 		level.Error(util_log.Logger).Log("msg", "error getting delete request from the store", "err", err)
-		serverutil.JSONError(w, http.StatusInternalServerError, err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	if deleteRequest == nil {
-		serverutil.JSONError(w, http.StatusBadRequest, "could not find delete request with given id")
+		http.Error(w, "could not find delete request with given id", http.StatusBadRequest)
 		return
 	}
 
 	if deleteRequest.Status != StatusReceived {
-		serverutil.JSONError(w, http.StatusBadRequest, "deletion of request which is in process or already processed is not allowed")
+		http.Error(w, "deletion of request which is in process or already processed is not allowed", http.StatusBadRequest)
 		return
 	}
 
 	if deleteRequest.CreatedAt.Add(dm.deleteRequestCancelPeriod).Before(model.Now()) {
-		serverutil.JSONError(w, http.StatusBadRequest, "deletion of request past the deadline of %s since its creation is not allowed", dm.deleteRequestCancelPeriod.String())
+		http.Error(w, fmt.Sprintf("deletion of request past the deadline of %s since its creation is not allowed", dm.deleteRequestCancelPeriod.String()), http.StatusBadRequest)
 		return
 	}
 
 	if err := dm.deleteRequestsStore.RemoveDeleteRequest(ctx, userID, requestID, deleteRequest.CreatedAt, deleteRequest.StartTime, deleteRequest.EndTime); err != nil {
 		level.Error(util_log.Logger).Log("msg", "error cancelling the delete request", "err", err)
-		serverutil.JSONError(w, http.StatusInternalServerError, err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 

pkg/util/server/error.go

@@ -2,9 +2,7 @@ package server
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
-	"fmt"
 	"net/http"
 
 	"google.golang.org/grpc/codes"
@@ -28,26 +26,6 @@ const (
 	ErrDeadlineExceeded = "Request timed out, decrease the duration of the request or add more label matchers (prefer exact match over regex match) to reduce the amount of data processed."
 )
 
-type ErrorResponseBody struct {
-	Code    int    `json:"code"`
-	Status  string `json:"status"`
-	Message string `json:"message"`
-}
-
-func NotFoundHandler(w http.ResponseWriter, r *http.Request) {
-	JSONError(w, 404, "not found")
-}
-
-func JSONError(w http.ResponseWriter, code int, message string, args ...interface{}) {
-	w.Header().Set("Content-Type", "application/json; charset=utf-8")
-	w.WriteHeader(code)
-	_ = json.NewEncoder(w).Encode(ErrorResponseBody{
-		Code:    code,
-		Status:  "error",
-		Message: fmt.Sprintf(message, args...),
-	})
-}
-
 // WriteError write a go error with the correct status code.
 func WriteError(err error, w http.ResponseWriter) {
 	var (
@@ -57,31 +35,33 @@ func WriteError(err error, w http.ResponseWriter) {
 
 	me, ok := err.(util.MultiError)
 	if ok && me.Is(context.Canceled) {
-		JSONError(w, StatusClientClosedRequest, ErrClientCanceled)
+		http.Error(w, ErrClientCanceled, StatusClientClosedRequest)
 		return
 	}
 	if ok && me.IsDeadlineExceeded() {
-		JSONError(w, http.StatusGatewayTimeout, ErrDeadlineExceeded)
+		http.Error(w, ErrDeadlineExceeded, http.StatusGatewayTimeout)
 		return
 	}
 
 	s, isRPC := status.FromError(err)
 	switch {
 	case errors.Is(err, context.Canceled) ||
 		(errors.As(err, &promErr) && errors.Is(promErr.Err, context.Canceled)):
-		JSONError(w, StatusClientClosedRequest, ErrClientCanceled)
+		http.Error(w, ErrClientCanceled, StatusClientClosedRequest)
 	case errors.Is(err, context.DeadlineExceeded) ||
 		(isRPC && s.Code() == codes.DeadlineExceeded):
-		JSONError(w, http.StatusGatewayTimeout, ErrDeadlineExceeded)
-	case errors.As(err, &queryErr),
-		errors.Is(err, logqlmodel.ErrLimit) || errors.Is(err, logqlmodel.ErrParse) || errors.Is(err, logqlmodel.ErrPipeline),
-		errors.Is(err, user.ErrNoOrgID):
-		JSONError(w, http.StatusBadRequest, err.Error())
+		http.Error(w, ErrDeadlineExceeded, http.StatusGatewayTimeout)
+	case errors.As(err, &queryErr):
+		http.Error(w, err.Error(), http.StatusBadRequest)
+	case errors.Is(err, logqlmodel.ErrLimit) || errors.Is(err, logqlmodel.ErrParse) || errors.Is(err, logqlmodel.ErrPipeline):
+		http.Error(w, err.Error(), http.StatusBadRequest)
+	case errors.Is(err, user.ErrNoOrgID):
+		http.Error(w, err.Error(), http.StatusBadRequest)
 	default:
 		if grpcErr, ok := httpgrpc.HTTPResponseFromError(err); ok {
-			JSONError(w, int(grpcErr.Code), string(grpcErr.Body))
+			http.Error(w, string(grpcErr.Body), int(grpcErr.Code))
 			return
 		}
-		JSONError(w, http.StatusInternalServerError, err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}
 }