diff --git a/pkg/util/errutil/errhttp/writer.go b/pkg/util/errutil/errhttp/writer.go index 165799a1d6b..5bbd9aede2e 100644 --- a/pkg/util/errutil/errhttp/writer.go +++ b/pkg/util/errutil/errhttp/writer.go @@ -8,12 +8,11 @@ import ( "reflect" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/util/errutil" ) var ErrNonGrafanaError = errutil.NewBase(errutil.StatusInternal, "core.MalformedError") -var defaultLogger = log.New("request-errors") +var defaultLogger = log.New("requestErrors") // ErrorOptions is a container for functional options passed to [Write]. type ErrorOptions struct { @@ -23,9 +22,9 @@ type ErrorOptions struct { // Write writes an error to the provided [http.ResponseWriter] with the // appropriate HTTP status and JSON payload from [errutil.Error]. -// Write also logs the provided error to either the contextlogger, -// the "request-errors" logger, or the logger provided as a functional -// option using [WithLogger]. +// Write also logs the provided error to either the "request-errors" +// logger, or the logger provided as a functional option using +// [WithLogger]. // When passing errors that are not [errors.As] compatible with // [errutil.Error], [ErrNonGrafanaError] will be used to create a // generic 500 Internal Server Error payload by default, this is @@ -45,8 +44,8 @@ func Write(ctx context.Context, err error, w http.ResponseWriter, opts ...func(E logError(ctx, gErr, opt) pub := gErr.Public() - w.WriteHeader(pub.StatusCode) w.Header().Add("Content-Type", "application/json") + w.WriteHeader(pub.StatusCode) err = json.NewEncoder(w).Encode(pub) if err != nil { defaultLogger.FromContext(ctx).Error("error while writing error", "error", err) @@ -68,9 +67,6 @@ func WithLogger(opt ErrorOptions, logger log.Logger) ErrorOptions { func logError(ctx context.Context, e errutil.Error, opt ErrorOptions) { var logger log.Logger = defaultLogger - if reqCtx := contexthandler.FromContext(ctx); reqCtx != nil && reqCtx.Logger != nil { - logger = reqCtx.Logger - } if opt.logger != nil { logger = opt.logger } diff --git a/pkg/web/context.go b/pkg/web/context.go index 897aef9db64..ca878c7ccd4 100644 --- a/pkg/web/context.go +++ b/pkg/web/context.go @@ -22,6 +22,9 @@ import ( "net/url" "strconv" "strings" + + "github.com/grafana/grafana/pkg/util/errutil" + "github.com/grafana/grafana/pkg/util/errutil/errhttp" ) // Context represents the runtime context of current request of Macaron instance. @@ -34,6 +37,8 @@ type Context struct { template *template.Template } +var errMissingWrite = errutil.NewBase(errutil.StatusInternal, "web.missingWrite") + func (ctx *Context) run() { h := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) for i := len(ctx.mws) - 1; i >= 0; i-- { @@ -47,7 +52,11 @@ func (ctx *Context) run() { // This indicates nearly always that a middleware is misbehaving and not calling its next.ServeHTTP(). // In rare cases where a blank http.StatusOK without any body is wished, explicitly state that using w.WriteStatus(http.StatusOK) if !rw.Written() { - panic("chain did not write HTTP response") + errhttp.Write( + ctx.Req.Context(), + errMissingWrite.Errorf("chain did not write HTTP response: %s", ctx.Req.URL.Path), + rw, + ) } } diff --git a/pkg/web/context_test.go b/pkg/web/context_test.go index 1144df15bfd..da9c5edaa14 100644 --- a/pkg/web/context_test.go +++ b/pkg/web/context_test.go @@ -2,8 +2,11 @@ package web import ( "net/http" + "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" + "github.com/grafana/grafana/pkg/infra/log" ) @@ -94,3 +97,17 @@ func TestContext_RemoteAddr(t *testing.T) { }) } } + +func TestContext_noHandler(t *testing.T) { + recorder := httptest.NewRecorder() + + method := http.MethodGet + c := &Context{ + Req: httptest.NewRequest(method, "/", nil), + Resp: NewResponseWriter(method, recorder), + } + + c.run() + + assert.Equal(t, http.StatusInternalServerError, recorder.Code) +}