Add error handling for ForEach's.

cmd_mirror.go

@@ -18,8 +18,9 @@ func aptlyMirrorList(cmd *commander.Command, args []string) error {
 	fmt.Printf("List of mirrors:\n")
 
 	repoCollection := debian.NewRemoteRepoCollection(context.database)
-	repoCollection.ForEach(func(repo *debian.RemoteRepo) {
+	repoCollection.ForEach(func(repo *debian.RemoteRepo) error {
 		fmt.Printf(" * %s\n", repo)
+		return nil
 	})
 
 	fmt.Printf("\nTo get more information about repository, run `aptly mirror show <name>`.\n")

cmd_snapshot.go

@@ -40,7 +40,7 @@ func aptlySnapshotCreate(cmd *commander.Command, args []string) error {
 		return fmt.Errorf("unable to add snapshot: %s", err)
 	}
 
-	fmt.Printf("\nSnapshot %s successfully created.\nYou can run 'aptly snapshot publish %s' to publish snapshot as Debian repository.\n", snapshot.Name, snapshot.Name)
+	fmt.Printf("\nSnapshot %s successfully created.\nYou can run 'aptly publish snapshot %s' to publish snapshot as Debian repository.\n", snapshot.Name, snapshot.Name)
 
 	return err
 }
@@ -55,8 +55,9 @@ func aptlySnapshotList(cmd *commander.Command, args []string) error {
 	fmt.Printf("List of snapshots:\n")
 
 	snapshotCollection := debian.NewSnapshotCollection(context.database)
-	snapshotCollection.ForEach(func(snapshot *debian.Snapshot) {
+	snapshotCollection.ForEach(func(snapshot *debian.Snapshot) error {
 		fmt.Printf(" * %s\n", snapshot)
+		return nil
 	})
 
 	fmt.Printf("\nTo get more information about snapshot, run `aptly snapshot show <name>`.\n")
@@ -91,10 +92,17 @@ func aptlySnapshotShow(cmd *commander.Command, args []string) error {
 
 	packageCollection := debian.NewPackageCollection(context.database)
 
-	snapshot.RefList().ForEach(func(key []byte) {
-		p, _ := packageCollection.ByKey(key)
+	err = snapshot.RefList().ForEach(func(key []byte) error {
+		p, err := packageCollection.ByKey(key)
+		if err != nil {
+			return err
+		}
 		fmt.Printf("  %s\n", p)
+		return nil
 	})
+	if err != nil {
+		return fmt.Errorf("unable to load packages: %s", err)
+	}
 
 	return err
 }

debian/list.go

@@ -35,12 +35,15 @@ func (l *PackageList) Add(p *Package) error {
 }
 
 // ForEach calls handler for each package in list
-//
-// TODO: Error handling
-func (l *PackageList) ForEach(handler func(*Package)) {
+func (l *PackageList) ForEach(handler func(*Package) error) error {
+	var err error
 	for _, p := range l.packages {
-		handler(p)
+		err = handler(p)
+		if err != nil {
+			return err
+		}
 	}
+	return err
 }
 
 // Len returns number of packages in the list
@@ -105,10 +108,13 @@ func (l *PackageRefList) Decode(input []byte) error {
 }
 
 // ForEach calls handler for each package ref in list
-//
-// TODO: Error handling
-func (l *PackageRefList) ForEach(handler func([]byte)) {
+func (l *PackageRefList) ForEach(handler func([]byte) error) error {
+	var err error
 	for _, p := range l.Refs {
-		handler(p)
+		err = handler(p)
+		if err != nil {
+			return err
+		}
 	}
+	return err
 }

debian/list_test.go

@@ -1,6 +1,7 @@
 package debian
 
 import (
+	"errors"
 	. "launchpad.net/gocheck"
 )
 
@@ -46,11 +47,22 @@ func (s *PackageListSuite) TestForeach(c *C) {
 	s.list.Add(s.p3)
 
 	Len := 0
-	s.list.ForEach(func(*Package) {
+	err := s.list.ForEach(func(*Package) error {
 		Len++
+		return nil
 	})
 
 	c.Check(Len, Equals, 2)
+	c.Check(err, IsNil)
+
+	e := errors.New("a")
+
+	err = s.list.ForEach(func(*Package) error {
+		return e
+	})
+
+	c.Check(err, Equals, e)
+
 }
 
 func (s *PackageListSuite) TestNewPackageRefList(c *C) {
@@ -91,9 +103,19 @@ func (s *PackageListSuite) TestPackageRefListForeach(c *C) {
 	reflist := NewPackageRefListFromPackageList(s.list)
 
 	Len := 0
-	reflist.ForEach(func([]byte) {
+	err := reflist.ForEach(func([]byte) error {
 		Len++
+		return nil
 	})
 
 	c.Check(Len, Equals, 4)
+	c.Check(err, IsNil)
+
+	e := errors.New("b")
+
+	err = reflist.ForEach(func([]byte) error {
+		return e
+	})
+
+	c.Check(err, Equals, e)
 }

debian/publish.go

@@ -50,10 +50,16 @@ func (p *PublishedRepo) Publish(repo *Repository, packageCollection *PackageColl
 	// Load all packages
 	list := NewPackageList()
 
-	p.snapshot.RefList().ForEach(func(key []byte) {
-		pkg, _ := packageCollection.ByKey(key)
-		list.Add(pkg)
+	err = p.snapshot.RefList().ForEach(func(key []byte) error {
+		pkg, err := packageCollection.ByKey(key)
+		if err != nil {
+			return err
+		}
+		return list.Add(pkg)
 	})
+	if err != nil {
+		return fmt.Errorf("unable to load packages: %s", err)
+	}
 
 	if list.Len() == 0 {
 		return fmt.Errorf("repository is empty, can't publish")
@@ -61,10 +67,11 @@ func (p *PublishedRepo) Publish(repo *Repository, packageCollection *PackageColl
 
 	if p.Architectures == nil {
 		p.Architectures = make([]string, 0, 10)
-		list.ForEach(func(pkg *Package) {
+		list.ForEach(func(pkg *Package) error {
 			if pkg.Architecture != "all" && !utils.StrSliceHasItem(p.Architectures, pkg.Architecture) {
 				p.Architectures = append(p.Architectures, pkg.Architecture)
 			}
+			return nil
 		})
 	}
 
@@ -89,18 +96,33 @@ func (p *PublishedRepo) Publish(repo *Repository, packageCollection *PackageColl
 
 		bufWriter := bufio.NewWriter(packagesFile)
 
-		list.ForEach(func(pkg *Package) {
+		err = list.ForEach(func(pkg *Package) error {
 			if pkg.Architecture == arch || pkg.Architecture == "all" {
-				path, _ := repo.LinkFromPool(p.Prefix, p.Component, pkg.Filename, pkg.HashMD5, pkg.Source)
+				path, err := repo.LinkFromPool(p.Prefix, p.Component, pkg.Filename, pkg.HashMD5, pkg.Source)
+				if err != nil {
+					return err
+				}
 
-				// TODO: error handling
 				pkg.Filename = path
-				pkg.Stanza().WriteTo(bufWriter)
-				bufWriter.WriteByte('\n')
+
+				err = pkg.Stanza().WriteTo(bufWriter)
+				if err != nil {
+					return err
+				}
+				err = bufWriter.WriteByte('\n')
+				if err != nil {
+					return err
+				}
 
 			}
+
+			return nil
 		})
 
+		if err != nil {
+			return fmt.Errorf("unable to creates process packages: %s", err)
+		}
+
 		err = bufWriter.Flush()
 		if err != nil {
 			return fmt.Errorf("unable to write Packages file: %s", err)

debian/remote.go

@@ -176,31 +176,50 @@ func (repo *RemoteRepo) Download(d utils.Downloader, packageCollection *PackageC
 	}
 
 	// Save package meta information to DB
-	list.ForEach(func(p *Package) {
-		packageCollection.Update(p)
+	err := list.ForEach(func(p *Package) error {
+		return packageCollection.Update(p)
 	})
 
+	if err != nil {
+		return fmt.Errorf("unable to save packages to db: %s", err)
+	}
+
 	// Download all package files
 	ch := make(chan error, list.Len())
 	count := 0
 
-	list.ForEach(func(p *Package) {
+	err = list.ForEach(func(p *Package) error {
 		poolPath, err := packageRepo.PoolPath(p.Filename, p.HashMD5)
-		if err == nil {
-			if !p.VerifyFile(poolPath) {
-				d.Download(repo.PackageURL(p.Filename).String(), poolPath, ch)
-				count++
-			}
+		if err != nil {
+			return err
 		}
+
+		if !p.VerifyFile(poolPath) {
+			d.Download(repo.PackageURL(p.Filename).String(), poolPath, ch)
+			count++
+		}
+		return nil
 	})
 
+	if err != nil {
+		return fmt.Errorf("unable to download packages: %s", err)
+	}
+
+	errors := make([]string, 0)
+
 	// Wait for all downloads to finish
-	// TODO: report errors
 	for count > 0 {
-		_ = <-ch
+		err = <-ch
+		if err != nil {
+			errors = append(errors, err.Error())
+		}
 		count--
 	}
 
+	if len(errors) > 0 {
+		return fmt.Errorf("download errors: %s", strings.Join(errors, ", "))
+	}
+
 	repo.LastDownloadDate = time.Now()
 	repo.packageRefs = NewPackageRefListFromPackageList(list)
 
@@ -331,8 +350,13 @@ func (collection *RemoteRepoCollection) ByUUID(uuid string) (*RemoteRepo, error)
 }
 
 // ForEach runs method for each repository
-func (collection *RemoteRepoCollection) ForEach(handler func(*RemoteRepo)) {
+func (collection *RemoteRepoCollection) ForEach(handler func(*RemoteRepo) error) error {
+	var err error
 	for _, r := range collection.list {
-		handler(r)
+		err = handler(r)
+		if err != nil {
+			return err
+		}
 	}
+	return err
 }

debian/remote_test.go

@@ -1,6 +1,7 @@
 package debian
 
 import (
+	"errors"
 	"github.com/smira/aptly/database"
 	"github.com/smira/aptly/utils"
 	. "launchpad.net/gocheck"
@@ -187,8 +188,19 @@ func (s *RemoteRepoCollectionSuite) TestForEach(c *C) {
 	s.collection.Add(repo)
 
 	count := 0
-	s.collection.ForEach(func(*RemoteRepo) { count++ })
+	err := s.collection.ForEach(func(*RemoteRepo) error {
+		count++
+		return nil
+	})
 	c.Assert(count, Equals, 1)
+	c.Assert(err, IsNil)
+
+	e := errors.New("c")
+
+	err = s.collection.ForEach(func(*RemoteRepo) error {
+		return e
+	})
+	c.Assert(err, Equals, e)
 }
 
 const exampleReleaseFile = `Origin: LP-PPA-agenda-developers-daily

debian/snapshot.go

@@ -162,8 +162,13 @@ func (collection *SnapshotCollection) ByName(name string) (*Snapshot, error) {
 }
 
 // ForEach runs method for each snapshot
-func (collection *SnapshotCollection) ForEach(handler func(*Snapshot)) {
+func (collection *SnapshotCollection) ForEach(handler func(*Snapshot) error) error {
+	var err error
 	for _, s := range collection.list {
-		handler(s)
+		err = handler(s)
+		if err != nil {
+			return err
+		}
 	}
+	return err
 }

debian/snapshot_test.go

@@ -1,6 +1,7 @@
 package debian
 
 import (
+	"errors"
 	"github.com/smira/aptly/database"
 	. "launchpad.net/gocheck"
 )
@@ -116,6 +117,16 @@ func (s *SnapshotCollectionSuite) TestForEach(c *C) {
 	s.collection.Add(s.snapshot2)
 
 	count := 0
-	s.collection.ForEach(func(*Snapshot) { count++ })
+	err := s.collection.ForEach(func(*Snapshot) error {
+		count++
+		return nil
+	})
 	c.Assert(count, Equals, 2)
+	c.Assert(err, IsNil)
+
+	e := errors.New("d")
+	err = s.collection.ForEach(func(*Snapshot) error {
+		return e
+	})
+	c.Assert(err, Equals, e)
 }