Pull request: all: imp err handling, logs

Updates AdguardTeam/AdGuardHome#3929.

Squashed commit of the following:

commit a15fb23
Author: Ainar Garipov <[email protected]>
Date:   Fri Dec 24 17:13:05 2021 +0300

    all: imp code, docs

commit a38cbd8
Author: Ainar Garipov <[email protected]>
Date:   Fri Dec 24 17:00:34 2021 +0300

    all: imp err handling, logs

.golangci.yml

@@ -36,7 +36,6 @@ linters:
     - dupl
     - gocyclo
     - goimports
-    - golint
     - gosec
     - misspell
     - stylecheck

proxy/config.go

@@ -2,11 +2,11 @@ package proxy
 
 import (
 	"crypto/tls"
-	"errors"
 	"net"
 	"time"
 
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/ameshkov/dnscrypt/v2"
 )
@@ -144,7 +144,7 @@ type Config struct {
 // validateConfig verifies that the supplied configuration is valid and returns an error if it's not
 func (p *Proxy) validateConfig() error {
 	if p.started {
-		return errors.New("server has been already started")
+		return errors.Error("server has been already started")
 	}
 
 	err := p.validateListenAddrs()
@@ -153,14 +153,15 @@ func (p *Proxy) validateConfig() error {
 	}
 
 	if p.UpstreamConfig == nil {
-		return errors.New("no default upstreams specified")
+		return errors.Error("no default upstreams specified")
 	}
 
 	if len(p.UpstreamConfig.Upstreams) == 0 {
 		if len(p.UpstreamConfig.DomainReservedUpstreams) == 0 {
-			return errors.New("no upstreams specified")
+			return errors.Error("no upstreams specified")
 		}
-		return errors.New("no default upstreams specified")
+
+		return errors.Error("no default upstreams specified")
 	}
 
 	if p.CacheMinTTL > 0 || p.CacheMaxTTL > 0 {
@@ -182,27 +183,30 @@ func (p *Proxy) validateConfig() error {
 	return nil
 }
 
-// validateListenAddrs -- checks if listen addrs are properly configured
+// validateListenAddrs returns an error if the addressses are not configured
+// properly.
 func (p *Proxy) validateListenAddrs() error {
 	if !p.hasListenAddrs() {
-		return errors.New("no listen address specified")
+		return errors.Error("no listen address specified")
 	}
 
-	if p.TLSListenAddr != nil && p.TLSConfig == nil {
-		return errors.New("cannot create a TLS listener without TLS config")
-	}
+	if p.TLSConfig == nil {
+		if p.TLSListenAddr != nil {
+			return errors.Error("cannot create tls listener without tls config")
+		}
 
-	if p.HTTPSListenAddr != nil && p.TLSConfig == nil {
-		return errors.New("cannot create an HTTPS listener without TLS config")
-	}
+		if p.HTTPSListenAddr != nil {
+			return errors.Error("cannot create https listener without tls config")
+		}
 
-	if p.QUICListenAddr != nil && p.TLSConfig == nil {
-		return errors.New("cannot create a QUIC listener without TLS config")
+		if p.QUICListenAddr != nil {
+			return errors.Error("cannot create quic listener without tls config")
+		}
 	}
 
 	if (p.DNSCryptTCPListenAddr != nil || p.DNSCryptUDPListenAddr != nil) &&
 		(p.DNSCryptResolverCert == nil || p.DNSCryptProviderName == "") {
-		return errors.New("cannot create a DNSCrypt listener without DNSCrypt config")
+		return errors.Error("cannot create dnscrypt listener without dnscrypt config")
 	}
 
 	return nil

proxy/dns64.go

@@ -1,11 +1,11 @@
 package proxy
 
 import (
-	"errors"
 	"fmt"
 	"net"
 
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/miekg/dns"
 )
@@ -66,7 +66,7 @@ func (p *Proxy) createDNS64MappedResponse(newAResp, oldAAAAResp *dns.Msg) (*dns.
 	p.nat64PrefixLock.Unlock()
 
 	if len(nat64Prefix) != NAT64PrefixLength {
-		return nil, errors.New("cannot create a mapped response, no NAT64 prefix specified")
+		return nil, errors.Error("cannot create a mapped response, no NAT64 prefix specified")
 	}
 
 	// check if there are no answers

proxy/errors.go

@@ -4,8 +4,9 @@
 package proxy
 
 import (
-	"errors"
 	"syscall"
+
+	"github.com/AdguardTeam/golibs/errors"
 )
 
 // isEPIPE checks if the underlying error is EPIPE.  syscall.EPIPE exists on all

proxy/errors_plan9.go

@@ -1,3 +1,4 @@
+//go:build plan9
 // +build plan9
 
 package proxy

proxy/errors_test.go

@@ -1,13 +1,14 @@
+//go:build !plan9
 // +build !plan9
 
 package proxy
 
 import (
-	"errors"
 	"fmt"
 	"syscall"
 	"testing"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -28,7 +29,7 @@ func TestIsEPIPE(t *testing.T) {
 		want: true,
 	}, {
 		name: "not_epipe",
-		err:  errors.New("test error"),
+		err:  errors.Error("test error"),
 		want: false,
 	}, {
 		name: "wrapped_epipe",

proxy/server.go

@@ -167,11 +167,10 @@ func (p *Proxy) respond(d *DNSContext) {
 
 	if err != nil {
 		if isNonCriticalError(err) {
-			// We're probably restarting, so log this with the debug
-			// level.
-			log.Debug("error while responding to a dns request: %s", err)
+			// We're probably restarting, so log this with the debug level.
+			log.Debug("responding to %s request: %s", d.Proto, err)
 		} else {
-			log.Printf("error while responding to a dns request: %s", err)
+			log.Info("responding to %s request: %s", d.Proto, err)
 		}
 	}
 }

proxy/server_tcp.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/dnsproxy/proxyutil"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/miekg/dns"
 )
@@ -50,11 +51,12 @@ func (p *Proxy) tcpPacketLoop(l net.Listener, proto Proto, requestGoroutinesSema
 		clientConn, err := l.Accept()
 
 		if err != nil {
-			if proxyutil.IsConnClosed(err) {
-				log.Tracef("TCP connection has been closed, exiting loop")
+			if errors.Is(err, net.ErrClosed) {
+				log.Debug("tcpPacketLoop: connection closed")
 			} else {
 				log.Info("got error when reading from TCP listen: %s", err)
 			}
+
 			break
 		} else {
 			requestGoroutinesSema.acquire()
@@ -134,11 +136,8 @@ func (p *Proxy) respondTCP(d *DNSContext) error {
 	}
 
 	err = proxyutil.WritePrefixed(bytes, conn)
-	if proxyutil.IsConnClosed(err) {
-		return err
-	}
-	if err != nil {
-		return fmt.Errorf("conn.Write(): %w", err)
+	if err != nil && !errors.Is(err, net.ErrClosed) {
+		return fmt.Errorf("writing message: %w", err)
 	}
 
 	return nil

proxy/server_udp.go

@@ -5,6 +5,7 @@ import (
 	"net"
 
 	"github.com/AdguardTeam/dnsproxy/proxyutil"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/miekg/dns"
 )
@@ -46,6 +47,7 @@ func (p *Proxy) udpCreate(udpAddr *net.UDPAddr) (*net.UDPConn, error) {
 	}
 
 	log.Info("Listening to udp://%s", udpListen.LocalAddr())
+
 	return udpListen, nil
 }
 
@@ -76,11 +78,12 @@ func (p *Proxy) udpPacketLoop(conn *net.UDPConn, requestGoroutinesSema semaphore
 			}()
 		}
 		if err != nil {
-			if proxyutil.IsConnClosed(err) {
-				log.Info("udpListen.ReadFrom() returned because we're reading from a closed connection, exiting loop")
+			if errors.Is(err, net.ErrClosed) {
+				log.Debug("udpPacketLoop: connection closed")
 			} else {
-				log.Info("got error when reading from UDP listen: %s", err)
+				log.Error("got error when reading from UDP listen: %s", err)
 			}
+
 			break
 		}
 	}
@@ -125,12 +128,12 @@ func (p *Proxy) respondUDP(d *DNSContext) error {
 	conn := d.Conn.(*net.UDPConn)
 	rAddr := d.Addr.(*net.UDPAddr)
 	n, err := proxyutil.UDPWrite(bytes, conn, rAddr, d.localIP)
-	if n == 0 && proxyutil.IsConnClosed(err) {
-		return err
-	}
-
 	if err != nil {
-		return fmt.Errorf("udpWrite(): %w", err)
+		if errors.Is(err, net.ErrClosed) {
+			return nil
+		}
+
+		return fmt.Errorf("writing message: %w", err)
 	}
 
 	if n != len(bytes) {

proxyutil/dns.go

@@ -2,16 +2,16 @@ package proxyutil
 
 import (
 	"encoding/binary"
-	"errors"
 	"fmt"
 	"io"
 	"net"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/miekg/dns"
 )
 
-// ErrTooLarge - DNS message is larger than 64kb
-var ErrTooLarge = errors.New("DNS message is too large")
+// ErrTooLarge means that a DNS message is larger than 64KiB.
+const ErrTooLarge errors.Error = "dns message is too large"
 
 // DNSSize returns if buffer size *advertised* in the requests OPT record.
 // Or when the request was over TCP, we return the maximum allowed size of 64K.

proxyutil/helpers.go

@@ -5,26 +5,17 @@ package proxyutil
 import (
 	"bytes"
 	"net"
-	"strings"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/miekg/dns"
 )
 
-// IsConnClosed - checks if the error signals of a closed server connecting
+// IsConnClosed returns true if the error signals of a closed server connecting.
+//
+// Deprecated: This function is deprecated.  Use errors.Is(err, net.ErrClosed)
+// instead.
 func IsConnClosed(err error) bool {
-	if err == nil {
-		return false
-	}
-	nerr, ok := err.(*net.OpError)
-	if !ok {
-		return false
-	}
-
-	if strings.Contains(nerr.Err.Error(), "use of closed network connection") {
-		return true
-	}
-
-	return false
+	return errors.Is(err, net.ErrClosed)
 }
 
 // GetIPFromDNSRecord - extracts IP address for a DNS record

upstream/upstream_dnscrypt.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/ameshkov/dnscrypt/v2"
 	"github.com/miekg/dns"
@@ -31,7 +32,7 @@ func (p *dnsCrypt) Address() string { return p.boot.URL.String() }
 func (p *dnsCrypt) Exchange(m *dns.Msg) (*dns.Msg, error) {
 	reply, err := p.exchangeDNSCrypt(m)
 
-	if os.IsTimeout(err) || err == io.EOF {
+	if errors.Is(err, os.ErrDeadlineExceeded) || errors.Is(err, io.EOF) {
 		// If request times out, it is possible that the server configuration has been changed.
 		// It is safe to assume that the key was rotated (for instance, as it is described here: https://dnscrypt.pl/2017/02/26/how-key-rotation-is-automated/).
 		// We should re-fetch the server certificate info so that the new requests were not failing.

upstream/upstream_doh.go

@@ -9,6 +9,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/miekg/dns"
 	"golang.org/x/net/http2"
 )
@@ -84,10 +85,11 @@ func (p *dnsOverHTTPS) exchangeHTTPSClient(m *dns.Msg, client *http.Client) (*dn
 		defer resp.Body.Close()
 	}
 	if err != nil {
-		// TODO: consider using errors.As
-		if os.IsTimeout(err) {
-			// If this is a timeout error, trying to forcibly re-create the HTTP client instance
-			// See https://github.com/AdguardTeam/AdGuardHome/issues/3217 for more details on this
+		if errors.Is(err, os.ErrDeadlineExceeded) {
+			// If this is a timeout error, trying to forcibly re-create the HTTP
+			// client instance.
+			//
+			// See https://github.com/AdguardTeam/AdGuardHome/issues/3217.
 			p.clientGuard.Lock()
 			p.client = nil
 			p.clientGuard.Unlock()

upstream/upstream_quic.go

@@ -2,12 +2,12 @@ package upstream
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net"
 	"sync"
 	"time"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/lucas-clemente/quic-go"
 	"github.com/miekg/dns"
 )
@@ -45,7 +45,7 @@ func (p *dnsOverQUIC) Exchange(m *dns.Msg) (*dns.Msg, error) {
 			// Check for EDNS TCP keepalive option
 			if option.Option() == dns.EDNS0TCPKEEPALIVE {
 				_ = session.CloseWithError(0, "") // Already closing the connection so we don't care about the error
-				return nil, errors.New("EDNS0 TCP keepalive option is set")
+				return nil, errors.Error("EDNS0 TCP keepalive option is set")
 			}
 		}
 	}