Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

client disconnecting while a query is being processed that requires proxying to graphite results in 502 #1820

Closed
fkaleo opened this issue May 6, 2020 · 2 comments · Fixed by #1821
Assignees
Labels
Milestone

Comments

@fkaleo
Copy link
Contributor

fkaleo commented May 6, 2020

Describe the bug
Issuing a query to metrictank that requires proxying to graphite leads to a 502 error being sent back to the client instead of 499 if the client disconnects too early.

Additional context
This is related to Go's proxying behaviour documented here golang/go#20071

Helpful Information
Metrictank Version: v0_13_1-700-gc2c86e4
Golang Version (if not using an official binary or docker image): 0.11.1
OS: Linux

@fkaleo fkaleo added the bug label May 6, 2020
@fkaleo fkaleo added this to the sprint-12 milestone May 6, 2020
@fkaleo
Copy link
Contributor Author

fkaleo commented May 6, 2020

This test program reproduces the issue:

package main

import (
	"fmt"
	"log"
	"net"
	"net/http"
	"net/http/httptest"
	"net/http/httputil"
	"net/url"
	"time"
)

type LoggingResponseWriter struct {
	http.ResponseWriter
}

func (rw *LoggingResponseWriter) Write(b []byte) (int, error) {
	 log.Println(string(b))
	return rw.ResponseWriter.Write(b)
}

func (rw *LoggingResponseWriter) WriteHeader(statusCode int) {
	 log.Println(statusCode)
	rw.ResponseWriter.WriteHeader(statusCode)
}

func main() {
	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		log.Printf("backend: %s", r.URL)
		w.Write([]byte("This is unexpected!"))
	}))
	defer backend.Close()

	backendURL, err := url.Parse(backend.URL)
	if err != nil {
		log.Fatalf("could not parse backend url %s: %s", backend.URL, err)
	}

	director := func(req *http.Request) {
		req.URL = &url.URL{}
		req.URL.Scheme = "http"
		req.URL.Host = backendURL.Host
		req.Host = ""
	}
	pxy := &httputil.ReverseProxy{Director: director}

	frontend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		log.Printf("proxying: %s", r.URL)
		w=&LoggingResponseWriter{w}
		pxy.ServeHTTP(w, r)
		log.Printf("boudiou: %s", r.URL)
	}))
	defer frontend.Close()

	frontendURL, err := url.Parse(frontend.URL)
	if err != nil {
		log.Fatalf("could not parse frontend url %s: %s", frontend.URL, err)
	}

	conn, err := net.DialTimeout("tcp", frontendURL.Host, 5*time.Second)
	if err != nil {
		log.Fatalf("could not dial: %s: %s", frontendURL.Host, err)
	}
	conn.Write([]byte("GET / HTTP/1.1\n"))
	conn.Write([]byte(fmt.Sprintf("Host: %s\n", frontendURL.Host)))
	conn.Write([]byte("\n"))
	err = conn.Close()
	// without this, the "http: proxy error: context canceled" message might not appear.
	time.Sleep(2 * time.Second)
}

@fkaleo
Copy link
Contributor Author

fkaleo commented May 6, 2020

Turns out it's in the documentation of ReverseProxy (https://golang.org/pkg/net/http/httputil/#ReverseProxy):

  // ErrorHandler is an optional function that handles errors
   // reaching the backend or errors from ModifyResponse.
   //
   // If nil, the default is to log the provided error and return
   // a 502 Status Bad Gateway response.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
1 participant