From d762d916ee6c9ca9db165a17c1c7d61f2a611605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Egelund-M=C3=BCller?= Date: Tue, 30 Jan 2024 18:32:09 +0100 Subject: [PATCH] Fix body close race condition for telemetry proxy (#3943) --- cli/pkg/local/app.go | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/cli/pkg/local/app.go b/cli/pkg/local/app.go index e6ba888b707..2e4f82a3bff 100644 --- a/cli/pkg/local/app.go +++ b/cli/pkg/local/app.go @@ -1,10 +1,12 @@ package local import ( + "bytes" "context" "encoding/json" "errors" "fmt" + "io" "net/http" "os" "path" @@ -427,22 +429,30 @@ func (a *App) trackingHandler(info *localInfo) http.Handler { return } + // Read entire body up front (since it may be closed before the request is sent in the goroutine below) + body, err := io.ReadAll(r.Body) + if err != nil { + a.BaseLogger.Info("failed to read telemetry request", zap.Error(err)) + w.WriteHeader(http.StatusOK) + return + } + + // Proxy request to rill intake + proxyReq, err := http.NewRequest(r.Method, "https://intake.rilldata.io/events/data-modeler-metrics", bytes.NewReader(body)) + if err != nil { + a.BaseLogger.Info("failed to create telemetry request", zap.Error(err)) + w.WriteHeader(http.StatusOK) + return + } + + // Copy the auth header + proxyReq.Header = http.Header{ + "Authorization": r.Header["Authorization"], + } + // Send event in the background to avoid blocking the frontend. // NOTE: If we stay with this telemetry approach, we should refactor and use ./cli/pkg/telemetry for batching and flushing events. go func() { - // Proxy request to rill intake - proxyReq, err := http.NewRequest(r.Method, "https://intake.rilldata.io/events/data-modeler-metrics", r.Body) - if err != nil { - a.BaseLogger.Info("failed to create telemetry request", zap.Error(err)) - w.WriteHeader(http.StatusOK) - return - } - - // Copy the auth header - proxyReq.Header = http.Header{ - "Authorization": r.Header["Authorization"], - } - // Send proxied request resp, err := http.DefaultClient.Do(proxyReq) if err != nil { @@ -450,7 +460,7 @@ func (a *App) trackingHandler(info *localInfo) http.Handler { w.WriteHeader(http.StatusOK) return } - defer resp.Body.Close() + resp.Body.Close() }() // Done