introduced go-multierror to handle multiple errors

0xERR0R · Oct 13, 2021 · cd76796 · cd76796
1 parent 57036aa
commit cd76796
Show file tree

Hide file tree

Showing 9 changed files with 59 additions and 68 deletions.
diff --git a/cmd/serve.go b/cmd/serve.go
@@ -44,7  44,7 @@ func startServer(_ *cobra.Command, _ []string) {
 	signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM)
 
 	srv, err := server.NewServer(config.GetConfig())
-	util.FatalOnError("cant start server: ", err...)
 	util.FatalOnError("cant start server: ", err)
 
 	srv.Start()
 

diff --git a/go.mod b/go.mod
@@ -41,6  41,8 @@ require (
 	github.com/go-openapi/swag v0.19.14 // indirect
 	github.com/go-sql-driver/mysql v1.6.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/hashicorp/errwrap v1.0.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/jinzhu/inflection v1.0.0 // indirect
 	github.com/jinzhu/now v1.1.2 // indirect

diff --git a/go.sum b/go.sum
@@ -193,11  193,14 @@ github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORR
 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g wF4CSlocrBnw=
 github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg FbDBtguAZLlVdkD9Q=
 github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
 github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3 sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH 1FKiLXxHSkmPseP kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
 github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
 github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
 github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0 lD48awMYo=
 github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
 github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z hiSfzSTQa6eBIzfwKfwNnHU=
 github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt ozgdvDeDU=
 github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4=

diff --git a/lists/list_cache.go b/lists/list_cache.go
@@ -13,6  13,8 @@ import (
 	"sync"
 	"time"
 
 	"github.com/hashicorp/go-multierror"
 
 	"github.com/0xERR0R/blocky/evt"
 	"github.com/0xERR0R/blocky/log"
 	"github.com/sirupsen/logrus"
@@ -81,7  83,7 @@ func (b *ListCache) Configuration() (result []string) {
 
 // NewListCache creates new list instance
 func NewListCache(t ListCacheType, groupToLinks map[string][]string, refreshPeriod time.Duration,
-	downloadTimeout time.Duration) (*ListCache, []error) {
 	downloadTimeout time.Duration) (*ListCache, error) {
 	groupCaches := make(map[string]cache)
 
 	b := &ListCache{
@@ -93,7  95,7 @@ func NewListCache(t ListCacheType, groupToLinks map[string][]string, refreshPeri
 	}
 	initError := b.refresh(true)
 
-	if len(initError) == 0 {
 	if initError == nil {
 		go periodicUpdate(b)
 	}
 
@@ -118,16  120,17 @@ func logger() *logrus.Entry {
 }
 
 type groupCache struct {
-	cache  []string
-	errors []error
 	cache []string
 	err   error
 }
 
 // downloads and reads files with domain names and creates cache for them
-func (b *ListCache) createCacheForGroup(links []string) (cache, []error) {
 func (b *ListCache) createCacheForGroup(links []string) (cache, error) {
 	var wg sync.WaitGroup
 
 	var err error
 
 	c := make(chan groupCache, len(links))
-	err := []error{}
 	// loop over links (http/local) or inline definitions
 	for _, link := range links {
 		wg.Add(1)
@@ -143,8  146,8 @@ Loop:
 	for {
 		select {
 		case res := <-c:
-			if len(res.errors) > 0 {
-				err = append(err, res.errors...)
 			if res.err != nil {
 				err = multierror.Append(err, res.err)
 			}
 			if res.cache == nil {
 				return nil, err
@@ -177,15  180,15 @@ func (b *ListCache) Match(domain string, groupsToCheck []string) (found bool, gr
 
 // Refresh triggers the refresh of a list
 func (b *ListCache) Refresh() {
-	b.refresh(false)
 	_ = b.refresh(false)
 }
-func (b *ListCache) refresh(init bool) []error {
-	res := []error{}
 func (b *ListCache) refresh(init bool) error {
 	var err error
 
 	for group, links := range b.groupToLinks {
-		cacheForGroup, errors := b.createCacheForGroup(links)
-		if len(errors) > 0 {
-			res = append(res, errors...)
 		cacheForGroup, e := b.createCacheForGroup(links)
 		if e != nil {
 			err = multierror.Append(err, multierror.Prefix(e, fmt.Sprintf("can't create cache group '%s':", group)))
 		}
 
 		if cacheForGroup != nil {
@@ -211,7  214,7 @@ func (b *ListCache) refresh(init bool) []error {
 		}
 	}
 
-	return res
 	return err
 }
 
 func (b *ListCache) downloadFile(link string) (io.ReadCloser, error) {
@@ -248,10  251,10 @@ func (b *ListCache) downloadFile(link string) (io.ReadCloser, error) {
 
 		if errors.As(err, &netErr) && (netErr.Timeout() || netErr.Temporary()) {
 			logger().WithField("link", link).WithField("attempt",
-				attempt).Warnf("Temporary network error / Timeout occurred, retrying... %s", netErr)
 				attempt).Warnf("Temporary network err / Timeout occurred, retrying... %s", netErr)
 		} else if errors.As(err, &dnsErr) {
 			logger().WithField("link", link).WithField("attempt",
-				attempt).Warnf("Name resolution error, retrying... %s", dnsErr.Err)
 				attempt).Warnf("Name resolution err, retrying... %s", dnsErr.Err)
 		}
 
 		time.Sleep(time.Second)
@@ -273,8  276,7 @@ func (b *ListCache) processFile(link string, ch chan<- groupCache, wg *sync.Wait
 	defer wg.Done()
 
 	result := groupCache{
-		cache:  []string{},
-		errors: []error{},
 		cache: []string{},
 	}
 
 	var r io.ReadCloser
@@ -284,13  286,13 @@ func (b *ListCache) processFile(link string, ch chan<- groupCache, wg *sync.Wait
 	r, err = b.getLinkReader(link)
 
 	if err != nil {
-		logger().Warn("error during file processing: ", err)
-		result.errors = append(result.errors, err)
 		logger().Warn("err during file processing: ", err)
 		result.err = multierror.Append(result.err, err)
 
 		var netErr net.Error
 
 		if errors.As(err, &netErr) && (netErr.Timeout() || netErr.Temporary()) {
-			// put nil to indicate the temporary error
 			// put nil to indicate the temporary err
 			result.cache = nil
 		}
 		ch <- result

diff --git a/lists/list_cache_test.go b/lists/list_cache_test.go
@@ -92,7  92,7 @@ var _ = Describe("ListCache", func() {
 				Expect(group).Should(Equal("gr1"))
 			})
 		})
-		When("a temporary error occurs on download", func() {
 		When("a temporary err occurs on download", func() {
 			var attempt uint64 = 1
 			It("should not delete existing elements from group cache", func() {
 				// should produce a timeout on second attempt
@@ -128,10  128,10 @@ var _ = Describe("ListCache", func() {
 				})
 			})
 		})
-		When("error occurs on download", func() {
 		When("err occurs on download", func() {
 			var attempt uint64 = 1
 			It("should delete existing elements from group cache", func() {
-				// should produce a 404 error on second attempt
 				// should produce a 404 err on second attempt
 				s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 					a := atomic.LoadUint64(&attempt)
 					if a != 1 {
@@ -149,7  149,7 @@ var _ = Describe("ListCache", func() {
 
 				sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
 				time.Sleep(time.Second)
-				By("Lists loaded without error", func() {
 				By("Lists loaded without err", func() {
 					found, group := sut.Match("blocked1.com", []string{"gr1"})
 					Expect(found).Should(BeTrue())
 					Expect(group).Should(Equal("gr1"))
@@ -158,7  158,7 @@ var _ = Describe("ListCache", func() {
 				sut.Refresh()
 				time.Sleep(time.Second)
 
-				By("List couldn't be loaded due to 404 error", func() {
 				By("List couldn't be loaded due to 404 err", func() {
 					found, _ := sut.Match("blocked1.com", []string{"gr1"})
 					Expect(found).Should(BeFalse())
 				})

diff --git a/resolver/blocking_resolver.go b/resolver/blocking_resolver.go
@@ -7,6  7,8 @@ import (
 	"strings"
 	"time"
 
 	"github.com/hashicorp/go-multierror"
 
 	"github.com/0xERR0R/blocky/api"
 	"github.com/0xERR0R/blocky/config"
 	"github.com/0xERR0R/blocky/evt"
@@ -80,21  82,21 @@ type BlockingResolver struct {
 }
 
 // NewBlockingResolver returns a new configured instance of the resolver
-func NewBlockingResolver(cfg config.BlockingConfig) (ChainedResolver, []error) {
 func NewBlockingResolver(cfg config.BlockingConfig) (ChainedResolver, error) {
 	blockHandler := createBlockHandler(cfg)
 	refreshPeriod := time.Duration(cfg.RefreshPeriod)
 	timeout := time.Duration(cfg.DownloadTimeout)
 	blacklistMatcher, blErr := lists.NewListCache(lists.ListCacheTypeBlacklist, cfg.BlackLists, refreshPeriod, timeout)
 	whitelistMatcher, wlErr := lists.NewListCache(lists.ListCacheTypeWhitelist, cfg.WhiteLists, refreshPeriod, timeout)
 	whitelistOnlyGroups := determineWhitelistOnlyGroups(&cfg)
 
-	var initErrors []error
-	if len(blErr) > 0 {
-		initErrors = append(initErrors, blErr...)
 	var err error
 	if blErr != nil {
 		err = multierror.Append(err, blErr)
 	}
 
-	if len(wlErr) > 0 {
-		initErrors = append(initErrors, wlErr...)
 	if wlErr != nil {
 		err = multierror.Append(err, wlErr)
 	}
 
 	res := &BlockingResolver{
@@ -109,7  111,7 @@ func NewBlockingResolver(cfg config.BlockingConfig) (ChainedResolver, []error) {
 		},
 	}
 
-	return res, initErrors
 	return res, multierror.Prefix(err, "blocking resolver: ")
 }
 
 // RefreshLists triggers the refresh of all black and white lists in the cache

diff --git a/server/server.go b/server/server.go
@@ -47,7  47,7 @@ func getServerAddress(addr string) string {
 }
 
 // NewServer creates new server instance with passed config
-func NewServer(cfg *config.Config) (server *Server, errors []error) {
 func NewServer(cfg *config.Config) (server *Server, err error) {
 	var dnsServers []*dns.Server
 
 	log.ConfigureLogger(cfg.LogLevel, cfg.LogFormat, cfg.LogTimestamp)
@@ -61,31  61,29 @@ func NewServer(cfg *config.Config) (server *Server, errors []error) {
 
 	var httpListener, httpsListener net.Listener
 
-	var err error
-
 	router := createRouter(cfg)
 
 	if cfg.HTTPPort != "" {
 		if httpListener, err = net.Listen("tcp", getServerAddress(cfg.HTTPPort)); err != nil {
-			return nil, append(errors, fmt.Errorf("start http listener on %s failed: %w", cfg.HTTPPort, err))
 			return nil, fmt.Errorf("start http listener on %s failed: %w", cfg.HTTPPort, err)
 		}
 
 		metrics.Start(router, cfg.Prometheus)
 	}
 
 	if cfg.HTTPSPort != "" {
 		if httpsListener, err = net.Listen("tcp", getServerAddress(cfg.HTTPSPort)); err != nil {
-			return nil, append(errors, fmt.Errorf("start https listener on port %s failed: %w", cfg.HTTPSPort, err))
 			return nil, fmt.Errorf("start https listener on port %s failed: %w", cfg.HTTPSPort, err)
 		}
 
 		metrics.Start(router, cfg.Prometheus)
 	}
 
 	metrics.RegisterEventListeners()
 
-	queryResolver, queryErrors := createQueryResolver(cfg)
-	if len(queryErrors) > 0 {
-		return nil, queryErrors
 	queryResolver, queryError := createQueryResolver(cfg)
 	if queryError != nil {
 		return nil, queryError
 	}
 
 	server = &Server{
@@ -104,7  102,7 @@ func NewServer(cfg *config.Config) (server *Server, errors []error) {
 
 	registerResolverAPIEndpoints(router, queryResolver)
 
-	return server, errors
 	return server, err
 }
 
 func registerResolverAPIEndpoints(router chi.Router, res resolver.Resolver) {
@@ -159,7  157,7 @@ func createUDPServer(address string) *dns.Server {
 		UDPSize: 65535}
 }
 
-func createQueryResolver(cfg *config.Config) (resolver.Resolver, []error) {
 func createQueryResolver(cfg *config.Config) (resolver.Resolver, error) {
 	br, brErr := resolver.NewBlockingResolver(cfg.Blocking)
 
 	return resolver.Chain(

diff --git a/server/server_test.go b/server/server_test.go
@@ -29,7  29,7 @@ var _ = Describe("Running DNS server", func() {
 		upstreamGoogle, upstreamFritzbox, upstreamClient config.Upstream
 		mockClientName                                   string
 		sut                                              *Server
-		err                                              []error
 		err                                              error
 		resp                                             *dns.Msg
 	)
 
@@ -108,7  108,7 @@ var _ = Describe("Running DNS server", func() {
 			},
 		})
 
-		Expect(err).Should(BeEmpty())
 		Expect(err).Should(Succeed())
 
 		// start server
 		go func() {
@@ -486,7  486,7 @@ var _ = Describe("Running DNS server", func() {
 					Port: ":55556",
 				})
 
-				Expect(err).Should(BeEmpty())
 				Expect(err).Should(Succeed())
 
 				// start server
 				go func() {
@@ -533,7  533,7 @@ var _ = Describe("Running DNS server", func() {
 					Port: "127.0.0.1:55557",
 				})
 
-				Expect(err).Should(BeEmpty())
 				Expect(err).Should(Succeed())
 
 				// start server
 				go func() {

diff --git a/util/common.go b/util/common.go
@@ -157,25  157,9 @@ func LogOnErrorWithEntry(logEntry *logrus.Entry, message string, err error) {
 }
 
 // FatalOnError logs the message only if error is not nil and exits the program execution
-func FatalOnError(message string, errors ...error) {
-	if errors != nil {
-		var nnErrors []error
-
-		for _, r := range errors {
-			if r != nil {
-				nnErrors = append(nnErrors, r)
-			}
-		}
-
-		if len(nnErrors) == 1 {
-			log.Log().Fatal(message, nnErrors[0])
-		} else if len(nnErrors) > 1 {
-			combined := []interface{}{message}
-			for _, r := range nnErrors {
-				combined = append(combined, "\n\t - ", r)
-			}
-			log.Log().Fatal(combined...)
-		}
 func FatalOnError(message string, err error) {
 	if err != nil {
 		log.Log().Fatal(message, err)
 	}
 }