diff --git a/core/metrics/prometheus.go b/core/metrics/prometheus.go index 0b89f85e..41248315 100644 --- a/core/metrics/prometheus.go +++ b/core/metrics/prometheus.go @@ -20,7 +20,7 @@ import ( type Metrics interface { WriteInitialMetrics(ctx context.Context) WriteAfterScanMetrics(ctx context.Context, success bool) - RecordRequest(ctx context.Context, endpoint, method, client string, status int, elapsed int64) + RecordRequest(ctx context.Context, endpoint, method, client string, status int32, elapsed int64) RecordPluginRequest(ctx context.Context, plugin, method string, ok bool, elapsed int64) GetHandler() http.Handler } @@ -56,7 +56,7 @@ func (m *metrics) WriteAfterScanMetrics(ctx context.Context, success bool) { getPrometheusMetrics().mediaScansCounter.With(scanLabels).Inc() } -func (m *metrics) RecordRequest(_ context.Context, endpoint, method, client string, status int, elapsed int64) { +func (m *metrics) RecordRequest(_ context.Context, endpoint, method, client string, status int32, elapsed int64) { httpLabel := prometheus.Labels{ "endpoint": endpoint, "method": method, @@ -233,7 +233,7 @@ func (n noopMetrics) WriteInitialMetrics(context.Context) {} func (n noopMetrics) WriteAfterScanMetrics(context.Context, bool) {} -func (n noopMetrics) RecordRequest(context.Context, string, string, string, int, int64) {} +func (n noopMetrics) RecordRequest(context.Context, string, string, string, int32, int64) {} func (n noopMetrics) RecordPluginRequest(context.Context, string, string, bool, int64) {} diff --git a/server/subsonic/api.go b/server/subsonic/api.go index 263fefb0..bb3d20e5 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -333,6 +333,7 @@ func sendResponse(w http.ResponseWriter, r *http.Request, payload *responses.Sub sendError(w, r, err) return } + if payload.Status == responses.StatusOK { if log.IsGreaterOrEqualTo(log.LevelTrace) { log.Debug(r.Context(), "API: Successful response", "endpoint", r.URL.Path, "status", "OK", "body", string(response)) @@ -342,6 +343,17 @@ func sendResponse(w http.ResponseWriter, r *http.Request, payload *responses.Sub } else { log.Warn(r.Context(), "API: Failed response", "endpoint", r.URL.Path, "error", payload.Error.Code, "message", payload.Error.Message) } + + statusPointer, ok := r.Context().Value(subsonicErrorPointer).(*int32) + + if ok && statusPointer != nil { + if payload.Status == responses.StatusOK { + *statusPointer = 0 + } else { + *statusPointer = payload.Error.Code + } + } + if _, err := w.Write(response); err != nil { log.Error(r, "Error sending response to client", "endpoint", r.URL.Path, "payload", string(response), err) } diff --git a/server/subsonic/api_test.go b/server/subsonic/api_test.go index 1658f094..eaecd7c0 100644 --- a/server/subsonic/api_test.go +++ b/server/subsonic/api_test.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/utils/gg" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "golang.org/x/net/context" ) var _ = Describe("sendResponse", func() { @@ -109,4 +110,18 @@ var _ = Describe("sendResponse", func() { }) }) + It("updates status pointer when an error occurs", func() { + pointer := int32(0) + + ctx := context.WithValue(r.Context(), subsonicErrorPointer, &pointer) + r = r.WithContext(ctx) + + payload.Status = responses.StatusFailed + payload.Error = &responses.Error{Code: responses.ErrorDataNotFound} + + sendResponse(w, r, payload) + Expect(w.Code).To(Equal(http.StatusOK)) + + Expect(pointer).To(Equal(responses.ErrorDataNotFound)) + }) }) diff --git a/server/subsonic/middlewares.go b/server/subsonic/middlewares.go index 4a0f327f..b8f01c83 100644 --- a/server/subsonic/middlewares.go +++ b/server/subsonic/middlewares.go @@ -226,21 +226,35 @@ func playerIDCookieName(userName string) string { return cookieName } +const subsonicErrorPointer = "subsonicErrorPointer" + func recordStats(metrics metrics.Metrics) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) + status := int32(-1) + contextWithStatus := context.WithValue(r.Context(), subsonicErrorPointer, &status) + start := time.Now() defer func() { + elapsed := time.Since(start).Milliseconds() + // We want to get the client name (even if not present for certain endpoints) p := req.Params(r) client, _ := p.String("c") - metrics.RecordRequest(r.Context(), strings.Replace(r.URL.Path, ".view", "", 1), r.Method, client, ww.Status(), time.Since(start).Milliseconds()) + // If there is no Subsonic status (e.g., HTTP 501 not implemented), fallback to HTTP + if status == -1 { + status = int32(ww.Status()) + } + + shortPath := strings.Replace(r.URL.Path, ".view", "", 1) + + metrics.RecordRequest(r.Context(), shortPath, r.Method, client, status, elapsed) }() - next.ServeHTTP(ww, r) + next.ServeHTTP(ww, r.WithContext(contextWithStatus)) } return http.HandlerFunc(fn) }