net: consistently return DNSError on lookup failure

Previously if we failed because the context timed out or was canceled,
we returned errCanceled or errTimeout.

Fixes #39178

Change-Id: I11bd7ebfaa7e5775fb05dfbb6b67f8307c8d8697
Reviewed-on: https://go-review.googlesource.com/c/go/+/353400
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/lookup.go b/src/net/lookup.go
index 3c0153028ce45a831f05e8bcae821de4991dc1f3..fe573b8a27914e27b7adfdb5528f7f515f262dd7 100644 (file)
--- a/src/net/lookup.go
+++ b/src/net/lookup.go
@@ -316,18 +316,39 @@ func (r *Resolver) lookupIPAddr(ctx context.Context, network, host string) ([]IP
                                lookupGroupCancel()
                        }()
                }
-               err := mapErr(ctx.Err())
+               ctxErr := ctx.Err()
+               err := &DNSError{
+                       Err:       mapErr(ctxErr).Error(),
+                       Name:      host,
+                       IsTimeout: ctxErr == context.DeadlineExceeded,
+               }
                if trace != nil && trace.DNSDone != nil {
                        trace.DNSDone(nil, false, err)
                }
                return nil, err
        case r := <-ch:
                lookupGroupCancel()
+               err := r.Err
+               if err != nil {
+                       if _, ok := err.(*DNSError); !ok {
+                               isTimeout := false
+                               if err == context.DeadlineExceeded {
+                                       isTimeout = true
+                               } else if terr, ok := err.(timeout); ok {
+                                       isTimeout = terr.Timeout()
+                               }
+                               err = &DNSError{
+                                       Err:       err.Error(),
+                                       Name:      host,
+                                       IsTimeout: isTimeout,
+                               }
+                       }
+               }
                if trace != nil && trace.DNSDone != nil {
                        addrs, _ := r.Val.([]IPAddr)
-                       trace.DNSDone(ipAddrsEface(addrs), r.Shared, r.Err)
+                       trace.DNSDone(ipAddrsEface(addrs), r.Shared, err)
                }
-               return lookupIPReturn(r.Val, r.Err, r.Shared)
+               return lookupIPReturn(r.Val, err, r.Shared)
        }
 }
 

diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go
index 3faaf007106b7c7327f1eb194dfc8d944f8b53d4..7ca6ac16ec67baf46348cd1dbbfca38960302ec1 100644 (file)
--- a/src/net/lookup_test.go
+++ b/src/net/lookup_test.go
@@ -890,7 +890,7 @@ func TestLookupContextCancel(t *testing.T) {
        ctx, ctxCancel := context.WithCancel(context.Background())
        ctxCancel()
        _, err := DefaultResolver.LookupIPAddr(ctx, "google.com")
-       if err != errCanceled {
+       if err.(*DNSError).Err != errCanceled.Error() {
                testenv.SkipFlakyNet(t)
                t.Fatal(err)
        }
@@ -1267,3 +1267,71 @@ func TestResolverLookupIP(t *testing.T) {
                })
        }
 }
+
+// A context timeout should still return a DNSError.
+func TestDNSTimeout(t *testing.T) {
+       origTestHookLookupIP := testHookLookupIP
+       defer func() { testHookLookupIP = origTestHookLookupIP }()
+       defer dnsWaitGroup.Wait()
+
+       timeoutHookGo := make(chan bool, 1)
+       timeoutHook := func(ctx context.Context, fn func(context.Context, string, string) ([]IPAddr, error), network, host string) ([]IPAddr, error) {
+               <-timeoutHookGo
+               return nil, context.DeadlineExceeded
+       }
+       testHookLookupIP = timeoutHook
+
+       checkErr := func(err error) {
+               t.Helper()
+               if err == nil {
+                       t.Error("expected an error")
+               } else if dnserr, ok := err.(*DNSError); !ok {
+                       t.Errorf("got error type %T, want %T", err, (*DNSError)(nil))
+               } else if !dnserr.IsTimeout {
+                       t.Errorf("got error %#v, want IsTimeout == true", dnserr)
+               } else if isTimeout := dnserr.Timeout(); !isTimeout {
+                       t.Errorf("got err.Timeout() == %t, want true", isTimeout)
+               }
+       }
+
+       // Single lookup.
+       timeoutHookGo <- true
+       _, err := LookupIP("golang.org")
+       checkErr(err)
+
+       // Double lookup.
+       var err1, err2 error
+       var wg sync.WaitGroup
+       wg.Add(2)
+       go func() {
+               defer wg.Done()
+               _, err1 = LookupIP("golang1.org")
+       }()
+       go func() {
+               defer wg.Done()
+               _, err2 = LookupIP("golang1.org")
+       }()
+       close(timeoutHookGo)
+       wg.Wait()
+       checkErr(err1)
+       checkErr(err2)
+
+       // Double lookup with context.
+       timeoutHookGo = make(chan bool)
+       ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+       wg.Add(2)
+       go func() {
+               defer wg.Done()
+               _, err1 = DefaultResolver.LookupIPAddr(ctx, "golang2.org")
+       }()
+       go func() {
+               defer wg.Done()
+               _, err2 = DefaultResolver.LookupIPAddr(ctx, "golang2.org")
+       }()
+       time.Sleep(10 * time.Nanosecond)
+       close(timeoutHookGo)
+       wg.Wait()
+       checkErr(err1)
+       checkErr(err2)
+       cancel()
+}