Bug [net/http]: client.Timeout exceeded errors are not wrapped

[yunginnanet]

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

$ go version
go version go1.21.1 linux/amd64

issue also exists in master branch

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GO111MODULE='auto'
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/kayos/.cache/go-build'
GOENV='/home/kayos/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/kayos/go/pkg/mod'
GOOS='linux'
GOPATH='/home/kayos/go'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOMOD=''
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build464817929=/tmp/go-build -gno-record-gcc-switches'

What did you do?

if _, err = GetPublicIP(); !errors.Is(err, context.DeadlineExceeded) {
	t.Errorf("expected error to be %v, got %v", context.DeadlineExceeded, 
}

from: gravitl/netmaker#2616

Full Context

package servercfg

import (
	"context"
	"errors"
	"net/http"
	"net/http/httptest"
	"os"
	"strings"
	"testing"
	"time"

	"github.com/gravitl/netmaker/config"
)

func TestGetPublicIP(t *testing.T) {
	var testIP = "55.55.55.55"
	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if _, err := w.Write([]byte(testIP)); err != nil {
			t.Errorf("expected no error, got %v", err)
		}
	}))
	defer server.Close()
	if err := os.Setenv("NETMAKER_TEST_IP_SERVICE", server.URL); err != nil {
		t.Logf("WARNING: could not set NETMAKER_TEST_IP_SERVICE env var")
	}

	t.Run("Use PUBLIC_IP_SERVICE if set", func(t *testing.T) {

		// set the environment variable
		if err := os.Setenv("PUBLIC_IP_SERVICE", server.URL); err != nil {
			t.Fatalf("expected no error, got %v", err)
		}
		defer func() {
			_ = os.Unsetenv("PUBLIC_IP_SERVICE")
		}()

		var ip string
		var err error
		if ip, err = GetPublicIP(); err != nil {
			t.Fatalf("expected no error, got %v", err)
		}
		if !strings.EqualFold(ip, testIP) {
			t.Errorf("expected IP to be %s, got %s", testIP, ip)
		}
	})

	t.Run("Use config.Config.Server.PublicIPService if PUBLIC_IP_SERVICE isn't set", func(t *testing.T) {
		config.Config.Server.PublicIPService = server.URL
		defer func() { config.Config.Server.PublicIPService = "" }()

		ip, err := GetPublicIP()
		if err != nil {
			t.Fatalf("expected no error, got %v", err)
		}
		if ip != testIP {
			t.Fatalf("expected IP to be %s, got %s", testIP, ip)
		}
	})

	t.Run("Handle service timeout", func(t *testing.T) {
		if os.Getenv("NETMAKER_TEST_IP_SERVICE") == "" {
			t.Skip("NETMAKER_TEST_IP_SERVICE not set")
		}

		var badTestIP = "123.45.67.91"
		badServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			// intentionally delay to simulate a timeout
			time.Sleep(3 * time.Second)
			_, _ = w.Write([]byte(badTestIP))
		}))
		defer badServer.Close()

		// we set this so that we can lower the timeout for the test to not hold up CI
		if err := os.Setenv("NETMAKER_TEST_BAD_IP_SERVICE", badServer.URL); err != nil {
			// but if we can't set it, we can't test it
			t.Skip("failed to set NETMAKER_TEST_BAD_IP_SERVICE, skipping test because we won't timeout")
		}
		defer func() {
			_ = os.Unsetenv("NETMAKER_TEST_BAD_IP_SERVICE")
		}()

		// mock the config
		oldConfig := config.Config.Server.PublicIPService
		config.Config.Server.PublicIPService = badServer.URL
		defer func() { config.Config.Server.PublicIPService = oldConfig }()

		res, err := GetPublicIP()
		if err != nil {
			t.Errorf("GetPublicIP() fallback has failed: %v", err)
		}
		if strings.EqualFold(res, badTestIP) {
			t.Errorf("GetPublicIP() returned the response from the server that should have timed out: %v", res)
		}
		if !strings.EqualFold(res, testIP) {
			t.Errorf("GetPublicIP() did not fallback to the correct IP: %v", res)
		}

		t.Run("Assert error is passed down", func(t *testing.T) {
			oldConfig = config.Config.Server.PublicIPService
			oldEnv := os.Getenv("NETMAKER_TEST_IP_SERVICE")

			// make sure that even the fallback fails
			if err = os.Setenv("NETMAKER_TEST_IP_SERVICE", badServer.URL); err != nil {
				t.Skipf("could not set NETMAKER_TEST_IP_SERVICE env var")
			}
			defer func() { _ = os.Setenv("NETMAKER_TEST_IP_SERVICE", oldEnv) }()

			if _, err = GetPublicIP(); !errors.Is(err, context.DeadlineExceeded) {
				t.Errorf("expected error to be %v, got %v", context.DeadlineExceeded, err)
			}
		})
	})

}

What did you expect to see?

errors.Is(err, context.DeadlineExceeded) == true

What did you see instead?

    serverconf_test.go:110: expected error to be context deadline exceeded, got Get "http://127.0.0.1:38513": context deadline exceeded (Client.Timeout exceeded while awaiting headers)

Proposed Fix

master...yunginnanet:go:net-http-error-wrapping

[yunginnanet]

ohp looks like i duplicated #50856

[seankhliao]

Duplicate of #50856

[yunginnanet]

#63448