Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: expose dialing errors from net package #23827

Closed
szuecs opened this issue Feb 14, 2018 · 3 comments
Closed

net/http: expose dialing errors from net package #23827

szuecs opened this issue Feb 14, 2018 · 3 comments

Comments

@szuecs
Copy link

szuecs commented Feb 14, 2018

I would like to get meaningful errors from the net package. Right now I can not decide if I got an error while a connection is already established or if it's while creating a TCP or TLS handshake.
I want to be able to decide from an error, if I can retry the call or not. Possibly net.Error interface has to get a new function to decide this easily.

The code shows a workaround wrapping a net.Dialer to decide if the error happened was during Dial or an already established connection.

What version of Go are you using (go version)?

go version go1.9.4 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

    GOARCH="amd64"
    GOBIN="/home/sszuecs/go/bin"
    GOEXE=""
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"
    GOOS="linux"
    GOPATH="/home/sszuecs/go"
    GORACE=""
    GOROOT="/usr/share/go"
    GOTOOLDIR="/usr/share/go/pkg/tool/linux_amd64"
    GCCGO="gccgo"
    CC="gcc"
    GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build480041304=/tmp/go-build -gno-record-gcc-switches"
    CXX="g++"
    CGO_ENABLED="1"
    CGO_CFLAGS="-g -O2"
    CGO_CPPFLAGS=""
    CGO_CXXFLAGS="-g -O2"
    CGO_FFLAGS="-g -O2"
    CGO_LDFLAGS="-g -O2"
    PKG_CONFIG="pkg-config"

What did you do?

package main

import (
	"context"
	"fmt"
	"log"
	"net"
	"net/http"
	"time"
)

type proxyError struct {
	err           error
	code          int
	dialingFailed bool
}

func (e proxyError) Error() string {
	return fmt.Sprintf("proxyError: %v", e.err)
}

type myDialer struct {
	net.Dialer
	f func(ctx context.Context, network, addr string) (net.Conn, error)
}

func myDialerNew(d net.Dialer) *myDialer {
	return &myDialer{
		Dialer: d,
		f:      d.DialContext,
	}
}

func (dc *myDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) {
	con, err := dc.f(ctx, network, addr)
	if err != nil {
		return nil, proxyError{
			err:           err,
			dialingFailed: true,
		}
	}
	return con, nil
}

func main() {

	tr := &http.Transport{
		DialContext: myDialerNew(net.Dialer{
			Timeout:   100 * time.Millisecond,
			KeepAlive: 30 * time.Second,
			DualStack: true,
		}).DialContext,
		TLSHandshakeTimeout:   200 * time.Millisecond,
		ResponseHeaderTimeout: 500 * time.Millisecond,
		IdleConnTimeout:       5 * time.Second,
	}

	go func(rt http.RoundTripper) {
		for {
			time.Sleep(1 * time.Second)

			req, err := http.NewRequest("GET", "http://127.0.0.1:10000/", nil)
			if err != nil {
				log.Printf("Failed to create request: %v", err)
				continue
			}

			resp, err := rt.RoundTrip(req)
			if err != nil {
				if perr, ok := err.(proxyError); ok {
					if nerr, ok := perr.err.(interface {
						Temporary() bool
						Timeout() bool
					}); ok {
						log.Printf("Failed to do roundtrip %v %v: %v", nerr.Temporary(), nerr.Timeout(), perr.err)
					}
					log.Printf("dial error: %v", perr)
				} else {
					if nerr, ok := err.(interface {
						Temporary() bool
						Timeout() bool
					}); ok {
						log.Printf("Failed to do roundtrip %v %v: %v", nerr.Temporary(), nerr.Timeout(), err)
					}
					log.Printf("not a dial error: %v", err)
				}
				continue
			}
			resp.Body.Close()
			log.Printf("resp status: %v", resp.Status)
		}
	}(tr)

	ch := make(chan struct{})
	<-ch
}

What did you expect to see?

I would like to do:

if nerr, ok := err.(net.Error); ok {
    if nerr.DialError() {
        retrySend(req)
    }
}

What did you see instead?

net.Error has Temporary() and Timeout() defined and rest of the errors are "untyped" fmt.Errorf("connection refused").

I would like to be able to dispatch by retrieable error. In an http proxy/loadbalancer this means you want to decide if you sent data or not, because if you already sent data, you should not retry.
I have to built a workaround as shown above in zalando/skipper@d3fa9e9 .

@artyom
Copy link
Member

artyom commented Feb 14, 2018

I want to be able to decide from an error, if I can retry the call or not. Possibly net.Error interface has to get a new function to decide this easily.

@szuecs just to make it clear, the Temporary() bool method of net.Error is not enough for you to decide whether you can retry? See for example: https://play.golang.org/p/pYCJ-Tfq9pV

Running this example locally produce:

~ ¶ go run /tmp/dial.go 
non-retryable
2018/02/14 22:39:56 dial tcp 127.0.0.1:10000: connect: connection refused

so the "connection refused" returned by Dial definitely implements net.Error interface.

@bradfitz
Copy link
Contributor

I would like to get meaningful errors from the net package. Right now I can not decide if I got an error while a connection is already established or if it's while creating a TCP or TLS handshake.

If you got the error from net.Dial, it was due to establishing the connection.

If you got the error from Read or Write, it was from an already established connection.

It seems that your bug report is actually about net/http, not about the net package, as stated in your bug title.

Marking this as a duplicate of #14203.

@szuecs
Copy link
Author

szuecs commented Feb 15, 2018

FYI: I changed my example code to show the problem more clearly.

@artyom, @bradfitz maybe it's more a documentation issue for the meaning of Temporary(). My tests can not show, that Temporary fails, but it seems a bit weird to me. I also looked into the Go src code in $GOROOT and I see there some cases, which I would do a retry, for example tlsHandshakeTimeoutError is retrieable from the point of layer7 protocols view, but returns true in Temporary():

% grep -rn 'Temporary() bool' $GOROOT/src/net 
/usr/share/go/src/net/cgo_windows.go:12:func (eai addrinfoErrno) Temporary() bool { return false }
/usr/share/go/src/net/http/transport.go:1869:func (e *httpError) Temporary() bool { return true }
/usr/share/go/src/net/http/transport.go:2202:func (tlsHandshakeTimeoutError) Temporary() bool { return true }
/usr/share/go/src/net/http/h2_bundle.go:3529:func (e *http2httpError) Temporary() bool { return true }
/usr/share/go/src/net/cgo_unix.go:33:func (eai addrinfoErrno) Temporary() bool { return eai == C.EAI_AGAIN }
/usr/share/go/src/net/cgo_stub.go:16:func (eai addrinfoErrno) Temporary() bool { return false }
/usr/share/go/src/net/net.go:377:   Temporary() bool // Is the error temporary?
/usr/share/go/src/net/net.go:486:   Temporary() bool
/usr/share/go/src/net/net.go:489:func (e *OpError) Temporary() bool {
/usr/share/go/src/net/net.go:527:func (e *AddrError) Temporary() bool { return false }
/usr/share/go/src/net/net.go:533:func (e UnknownNetworkError) Temporary() bool { return false }
/usr/share/go/src/net/net.go:539:func (e InvalidAddrError) Temporary() bool { return false }
/usr/share/go/src/net/net.go:549:func (e *DNSConfigError) Temporary() bool { return false }
/usr/share/go/src/net/net.go:585:func (e *DNSError) Temporary() bool { return e.IsTimeout || e.IsTemporary }
/usr/share/go/src/net/url/url_test.go:1523:func (e *temporaryError) Temporary() bool { return e.temporary }
/usr/share/go/src/net/url/url.go:41:        Temporary() bool
/usr/share/go/src/net/url/url.go:44:func (e *Error) Temporary() bool {

In general it's not only about "connection refused", here some other examples with a backend socat-test and logs from running the go code:

slow backend (see the $(sleep 1) that is the value for header X-Foo):

% socat -d -d TCP-L:10000,reuseaddr,fork,crlf SYSTEM:"echo  \"\\\"HTTP/1.0 200 OK\\\nDocumentType: text/plain\\\n\\\ndate: \$\(date\)\\\nserver:\$SOCAT_SOCKADDR:\$SOCAT_SOCKPORT\\\nclient: \$SOCAT_PEERADDR:\$SOCAT_PEERPORT\\\nX-Foo: \$\(sleep 1\)\\\n\\\"\"; cat"

client logs for slow backend:

2018/02/15 10:35:45 Failed to do roundtrip true true: net/http: timeout awaiting response headers
2018/02/15 10:35:45 not a dial error: net/http: timeout awaiting response headers

client logs without having a backend

2018/02/15 10:32:38 Failed to do roundtrip false false: dial tcp 127.0.0.1:10000: getsockopt: connection refused
2018/02/15 10:32:38 dial error: proxyError: dial tcp 127.0.0.1:10000: getsockopt: connection refused

middlebox that silently drop packets (iptables -I INPUT -p tcp --dst 127.0.0.1 --dport 10000 -j DROP)

2018/02/15 10:38:47 Failed to do roundtrip true true: dial tcp 127.0.0.1:10000: i/o timeout
2018/02/15 10:38:47 dial error: proxyError: dial tcp 127.0.0.1:10000: i/o timeout

test tls handshake failures

TBD

@mikioh mikioh changed the title expose dialing errors from net package net/http: expose dialing errors from net package Feb 21, 2018
@golang golang locked and limited conversation to collaborators Feb 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants