Pull request: replace agherr with golibs' errors

Merge in DNS/adguard-home from golibs-errors to master

Squashed commit of the following:

commit 5aba278a31c5a213bd9e08273ce7277c57713b22
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon May 24 17:05:18 2021 +0300

    all: imp code

commit f447eb875b81779fa9e391d98c31c1eeba7ef323
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon May 24 15:33:45 2021 +0300

    replace agherr with golibs' errors

internal/home/authglinet.go

@@ -73,22 +73,27 @@ func glGetTokenDate(file string) uint32 {
 	f, err := os.Open(file)
 	if err != nil {
 		log.Error("os.Open: %s", err)
+
 		return 0
 	}
-	defer f.Close()
+	defer func() {
+		derr := f.Close()
+		if derr != nil {
+			log.Error("glinet: closing file: %s", err)
+		}
+	}()
 
-	fileReadCloser, err := aghio.LimitReadCloser(f, MaxFileSize)
+	fileReader, err := aghio.LimitReader(f, MaxFileSize)
 	if err != nil {
 		log.Error("creating limited reader: %s", err)
 
 		return 0
 	}
-	defer fileReadCloser.Close()
 
 	var dateToken uint32
 
 	// This use of ReadAll is now safe, because we limited reader.
-	bs, err := io.ReadAll(fileReadCloser)
+	bs, err := io.ReadAll(fileReader)
 	if err != nil {
 		log.Error("reading token: %s", err)
 

internal/home/clients.go

@@ -11,7 +11,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghstrings"
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
@@ -20,6 +19,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/querylog"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 )
 
@@ -438,11 +438,11 @@ func (clients *clientsContainer) FindRuntimeClient(ip string) (RuntimeClient, bo
 func (clients *clientsContainer) check(c *Client) (err error) {
 	switch {
 	case c == nil:
-		return agherr.Error("client is nil")
+		return errors.Error("client is nil")
 	case c.Name == "":
-		return agherr.Error("invalid name")
+		return errors.Error("invalid name")
 	case len(c.IDs) == 0:
-		return agherr.Error("id required")
+		return errors.Error("id required")
 	default:
 		// Go on.
 	}
@@ -570,14 +570,14 @@ func (clients *clientsContainer) Update(name string, c *Client) (err error) {
 
 	prev, ok := clients.list[name]
 	if !ok {
-		return agherr.Error("client not found")
+		return errors.Error("client not found")
 	}
 
 	// First, check the name index.
 	if prev.Name != c.Name {
 		_, ok = clients.list[c.Name]
 		if ok {
-			return agherr.Error("client already exists")
+			return errors.Error("client already exists")
 		}
 	}
 

internal/home/config.go

@@ -1,7 +1,6 @@
 package home
 
 import (
-	"errors"
 	"fmt"
 	"net"
 	"os"
@@ -14,6 +13,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/querylog"
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
 	"github.com/AdguardTeam/AdGuardHome/internal/version"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/google/renameio/maybe"
 	yaml "gopkg.in/yaml.v2"

internal/home/controlinstall.go

@@ -15,7 +15,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/golibs/log"
 )
@@ -271,7 +270,7 @@ func copyInstallSettings(dst, src *configuration) {
 const shutdownTimeout = 5 * time.Second
 
 func shutdownSrv(ctx context.Context, cancel context.CancelFunc, srv *http.Server) {
-	defer agherr.LogPanic("")
+	defer log.OnPanic("")
 
 	if srv == nil {
 		return

internal/home/controlupdate.go

@@ -3,7 +3,6 @@ package home
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"net/http"
 	"os"
 	"os/exec"
@@ -14,6 +13,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/AdGuardHome/internal/updater"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 )
 

internal/home/dns.go

@@ -8,12 +8,12 @@ import (
 	"path/filepath"
 	"strconv"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/AdGuardHome/internal/querylog"
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
 	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/ameshkov/dnscrypt/v2"
 	yaml "gopkg.in/yaml.v2"
@@ -207,14 +207,14 @@ func generateServerConfig() (newConf dnsforward.ServerConfig, err error) {
 
 func newDNSCrypt(hosts []net.IP, tlsConf tlsConfigSettings) (dnscc dnsforward.DNSCryptConfig, err error) {
 	if tlsConf.DNSCryptConfigFile == "" {
-		return dnscc, agherr.Error("no dnscrypt_config_file")
+		return dnscc, errors.Error("no dnscrypt_config_file")
 	}
 
 	f, err := os.Open(tlsConf.DNSCryptConfigFile)
 	if err != nil {
 		return dnscc, fmt.Errorf("opening dnscrypt config: %w", err)
 	}
-	defer f.Close()
+	defer func() { err = errors.WithDeferred(err, f.Close()) }()
 
 	rc := &dnscrypt.ResolverConfig{}
 	err = yaml.NewDecoder(f).Decode(rc)

internal/home/filter.go

@@ -2,7 +2,6 @@ package home
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
 	"hash/crc32"
 	"io"
@@ -17,6 +16,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 )
 
@@ -566,17 +566,18 @@ func (f *Filtering) updateIntl(filter *filter) (updated bool, err error) {
 		if err != nil {
 			return updated, fmt.Errorf("open file: %w", err)
 		}
+		defer func() { err = errors.WithDeferred(err, f.Close()) }()
 
-		defer f.Close()
 		reader = f
 	} else {
 		var resp *http.Response
 		resp, err = Context.client.Get(filter.URL)
 		if err != nil {
 			log.Printf("Couldn't request filter from URL %s, skipping: %s", filter.URL, err)
+
 			return updated, err
 		}
-		defer resp.Body.Close()
+		defer func() { err = errors.WithDeferred(err, resp.Body.Close()) }()
 
 		if resp.StatusCode != http.StatusOK {
 			log.Printf("Got status code %d from URL %s, skipping", resp.StatusCode, filter.URL)
@@ -634,7 +635,7 @@ func (f *Filtering) load(filter *filter) (err error) {
 	} else if err != nil {
 		return fmt.Errorf("opening filter file: %w", err)
 	}
-	defer file.Close()
+	defer func() { err = errors.WithDeferred(err, file.Close()) }()
 
 	st, err := file.Stat()
 	if err != nil {

internal/home/home.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
-	"errors"
 	"fmt"
 	"io/fs"
 	"net"
@@ -21,7 +20,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
@@ -31,6 +29,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
 	"github.com/AdguardTeam/AdGuardHome/internal/updater"
 	"github.com/AdguardTeam/AdGuardHome/internal/version"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"gopkg.in/natefinch/lumberjack.v2"
 )
@@ -736,7 +735,7 @@ func customDialContext(ctx context.Context, network, addr string) (conn net.Conn
 		return conn, err
 	}
 
-	return nil, agherr.Many(fmt.Sprintf("couldn't dial to %s", addr), dialErrs...)
+	return nil, errors.List(fmt.Sprintf("couldn't dial to %s", addr), dialErrs...)
 }
 
 func getHTTPProxy(_ *http.Request) (*url.URL, error) {

internal/home/middlewares.go

@@ -1,6 +1,7 @@
 package home
 
 import (
+	"io"
 	"net/http"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghio"
@@ -58,14 +59,20 @@ func limitRequestBody(h http.Handler) (limited http.Handler) {
 			szLim = largerReqBodySzLim
 		}
 
-		r.Body, err = aghio.LimitReadCloser(r.Body, szLim)
+		var reader io.Reader
+		reader, err = aghio.LimitReader(r.Body, szLim)
 		if err != nil {
 			log.Error("limitRequestBody: %s", err)
 
 			return
 		}
 
-		h.ServeHTTP(w, r)
+		// HTTP handlers aren't supposed to call r.Body.Close(), so just
+		// replace the body in a clone.
+		rr := r.Clone(r.Context())
+		rr.Body = io.NopCloser(reader)
+
+		h.ServeHTTP(w, rr)
 	})
 }
 

internal/home/rdns.go

@@ -5,7 +5,6 @@ import (
 	"net"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
 	"github.com/AdguardTeam/golibs/cache"
 	"github.com/AdguardTeam/golibs/log"
@@ -85,7 +84,7 @@ func (r *RDNS) Begin(ip net.IP) {
 // workerLoop handles incoming IP addresses from ipChan and adds it into
 // clients.
 func (r *RDNS) workerLoop() {
-	defer agherr.LogPanic("rdns")
+	defer log.OnPanic("rdns")
 
 	for ip := range r.ipCh {
 		host, err := r.exchanger.Exchange(ip)

internal/home/rdns_test.go

@@ -3,7 +3,6 @@ package home
 import (
 	"bytes"
 	"encoding/binary"
-	"errors"
 	"net"
 	"sync"
 	"testing"
@@ -13,6 +12,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/AdguardTeam/dnsproxy/upstream"
 	"github.com/AdguardTeam/golibs/cache"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
@@ -141,7 +141,7 @@ func TestRDNS_WorkerLoop(t *testing.T) {
 		},
 	}
 	errUpstream := &aghtest.TestErrUpstream{
-		Err: errors.New("1234"),
+		Err: errors.Error("1234"),
 	}
 
 	testCases := []struct {

internal/home/service.go

@@ -1,7 +1,6 @@
 package home
 
 import (
-	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -11,6 +10,7 @@ import (
 	"syscall"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/kardianos/service"
 )

internal/home/tls.go

@@ -10,7 +10,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"encoding/pem"
-	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -21,6 +20,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"golang.org/x/sys/cpu"
 )
@@ -341,14 +341,14 @@ func verifyCertChain(data *tlsConfigStatus, certChain, serverName string) error
 		parsed, err := x509.ParseCertificate(cert.Bytes)
 		if err != nil {
 			data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err)
-			return errors.New(data.WarningValidation)
+			return errors.Error(data.WarningValidation)
 		}
 		parsedCerts = append(parsedCerts, parsed)
 	}
 
 	if len(parsedCerts) == 0 {
 		data.WarningValidation = "You have specified an empty certificate"
-		return errors.New(data.WarningValidation)
+		return errors.Error(data.WarningValidation)
 	}
 
 	data.ValidCert = true
@@ -415,14 +415,14 @@ func validatePkey(data *tlsConfigStatus, pkey string) error {
 
 	if key == nil {
 		data.WarningValidation = "No valid keys were found"
-		return errors.New(data.WarningValidation)
+		return errors.Error(data.WarningValidation)
 	}
 
 	// parse the decoded key
 	_, keytype, err := parsePrivateKey(key.Bytes)
 	if err != nil {
 		data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err)
-		return errors.New(data.WarningValidation)
+		return errors.Error(data.WarningValidation)
 	}
 
 	data.ValidKey = true
@@ -479,7 +479,7 @@ func parsePrivateKey(der []byte) (crypto.PrivateKey, string, error) {
 		case *ecdsa.PrivateKey:
 			return key, "ECDSA", nil
 		default:
-			return nil, "", errors.New("tls: found unknown private key type in PKCS#8 wrapping")
+			return nil, "", errors.Error("tls: found unknown private key type in PKCS#8 wrapping")
 		}
 	}
 
@@ -487,7 +487,7 @@ func parsePrivateKey(der []byte) (crypto.PrivateKey, string, error) {
 		return key, "ECDSA", nil
 	}
 
-	return nil, "", errors.New("tls: failed to parse private key")
+	return nil, "", errors.Error("tls: failed to parse private key")
 }
 
 // unmarshalTLS handles base64-encoded certificates transparently

internal/home/upgrade.go

@@ -1,7 +1,6 @@
 package home
 
 import (
-	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -12,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/google/renameio/maybe"
 	"golang.org/x/crypto/bcrypt"

internal/home/whois.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghio"
 	"github.com/AdguardTeam/golibs/cache"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 )
 
@@ -139,22 +140,22 @@ func whoisParse(data string) (m strmap) {
 const MaxConnReadSize = 64 * 1024
 
 // Send request to a server and receive the response
-func (w *Whois) query(ctx context.Context, target, serverAddr string) (string, error) {
+func (w *Whois) query(ctx context.Context, target, serverAddr string) (data string, err error) {
 	addr, _, _ := net.SplitHostPort(serverAddr)
 	if addr == "whois.arin.net" {
 		target = "n + " + target
 	}
+
 	conn, err := w.dialContext(ctx, "tcp", serverAddr)
 	if err != nil {
 		return "", err
 	}
-	defer conn.Close()
+	defer func() { err = errors.WithDeferred(err, conn.Close()) }()
 
-	connReadCloser, err := aghio.LimitReadCloser(conn, MaxConnReadSize)
+	r, err := aghio.LimitReader(conn, MaxConnReadSize)
 	if err != nil {
 		return "", err
 	}
-	defer connReadCloser.Close()
 
 	_ = conn.SetReadDeadline(time.Now().Add(time.Duration(w.timeoutMsec) * time.Millisecond))
 	_, err = conn.Write([]byte(target + "\r\n"))
@@ -163,12 +164,13 @@ func (w *Whois) query(ctx context.Context, target, serverAddr string) (string, e
 	}
 
 	// This use of ReadAll is now safe, because we limited the conn Reader.
-	data, err := io.ReadAll(connReadCloser)
+	var whoisData []byte
+	whoisData, err = io.ReadAll(r)
 	if err != nil {
 		return "", err
 	}
 
-	return string(data), nil
+	return string(whoisData), nil
 }
 
 // Query WHOIS servers (handle redirects)