Fix error handling in goroutine

feeder.go

@@ -1,21 +1,18 @@
 package feeder
 
 import (
-	"log"
 	"sync"
 	"time"
 
+	"github.com/pkg/errors"
+
 	ogp "github.com/otiai10/opengraph"
+	"golang.org/x/sync/errgroup"
 )
 
-// Deprecated: Fetcher is replaced by Crawler
-type Fetcher interface {
-	Fetch() (*Items, error)
-}
-
 // Crawler is interface for crawling
 type Crawler interface {
-	Fetch() (*Items, error)
+	Crawl() ([]*Item, error)
 }
 
 // Link represents http link
@@ -54,11 +51,6 @@ type Item struct {
 	Content   string
 }
 
-// Items represents slice of item
-type Items struct {
-	Items []*Item
-}
-
 // Feed represents rss feed or atom feed
 type Feed struct {
 	Title       string
@@ -69,55 +61,54 @@ type Feed struct {
 	Created     time.Time
 	Id          string
 	Subtitle    string
-	Items       Items
+	Items       []*Item
 	Copyright   string
 	Image       *Image
 }
 
-// Add adds item to Items
-func (items *Items) Add(i *Items) {
-	items.Items = append(items.Items, i.Items...)
-}
-
 // Crawl is function that crawls all site using goroutine.
-// func Crawl(fetchers ...Fetcher) *Items is deprecated
-func Crawl(crawlers ...Crawler) *Items {
-	items := &Items{}
+// func Crawl(crawlers ...Fetcher) *Items is deprecated
+func Crawl(crawlers ...Crawler) ([]*Item, error) {
+	items := []*Item{}
 	mutex := sync.Mutex{}
-	wg := sync.WaitGroup{}
 
+	eg := errgroup.Group{}
 	for _, f := range crawlers {
-		wg.Add(1)
-		go func(f Crawler) {
-			i, err := f.Fetch()
+		f := f
+		eg.Go(func() error {
+			i, err := f.Crawl()
 			if err != nil {
-				log.Fatal(err)
+				return err
 			} else {
 				mutex.Lock()
-				items.Add(i)
+				items = append(items, i...)
 				mutex.Unlock()
 			}
-			wg.Done()
-		}(f)
+			return nil
+		})
+	}
+	if err := eg.Wait(); err != nil {
+		return nil, errors.Wrap(err, "failed to crawl items")
 	}
-	wg.Wait()
 
-	fetchOGP(items)
+	items, err := fetchOGP(items)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to fetch ogp")
+	}
 
-	return items
+	return items, nil
 }
 
-func fetchOGP(items *Items) *Items {
-	wg := sync.WaitGroup{}
+func fetchOGP(items []*Item) ([]*Item, error) {
+	eg := errgroup.Group{}
 
-	for _, i := range items.Items {
-		wg.Add(1)
+	for _, i := range items {
 		i := i
-		go func() {
+		eg.Go(func() error {
 			if i.Enclosure == nil || i.Enclosure.URL == "" {
 				og, err := ogp.Fetch(i.Link.Href)
 				if err != nil {
-					log.Printf("Failed to fetch ogp. %#v", err)
+					return err
 				}
 
 				if len(og.Image) > 0 {
@@ -132,12 +123,10 @@ func fetchOGP(items *Items) *Items {
 					}
 					i.Enclosure.Length = "0"
 				}
-
 			}
-			wg.Done()
-		}()
+			return nil
+		})
 	}
-	wg.Wait()
 
-	return items
+	return items, nil
 }

feeder_test.go

@@ -14,13 +14,13 @@ type mockFetcher struct {
 	Id string
 }
 
-func (f *mockFetcher) Fetch() (*feeder.Items, error) {
+func (f *mockFetcher) Crawl() ([]*feeder.Item, error) {
 	sleepTime, _ := strconv.Atoi(f.Id)
 	time.Sleep(time.Second * time.Duration(sleepTime))
 
 	publishedString := "2019-01-01T00:00:00+09:00"
 	published, _ := time.Parse(time.RFC3339, publishedString)
-	return &feeder.Items{[]*feeder.Item{{
+	return []*feeder.Item{{
 		Title: "title",
 		Link: &feeder.Link{
 			Href: "http://ogp.me",
@@ -35,14 +35,14 @@ func (f *mockFetcher) Fetch() (*feeder.Items, error) {
 		Updated:     nil,
 		Created:     &published,
 		Content:     "",
-	}}}, nil
+	}}, nil
 }
 
 func TestCrawl(t *testing.T) {
 	publishedString := "2019-01-01T00:00:00+09:00"
 	published, _ := time.Parse(time.RFC3339, publishedString)
 
-	expected := &feeder.Items{[]*feeder.Item{{
+	expected := []*feeder.Item{{
 		Title: "title",
 		Link: &feeder.Link{
 			Href: "http://ogp.me",
@@ -56,12 +56,7 @@ func TestCrawl(t *testing.T) {
 		ID:          "1",
 		Updated:     nil,
 		Created:     &published,
-		Enclosure: &feeder.Enclosure{
-			URL:    "http://ogp.me/logo.png",
-			Type:   "image/png",
-			Length: "0",
-		},
-		Content: "",
+		Content:     "",
 	}, {
 		Title: "title",
 		Link: &feeder.Link{
@@ -76,21 +71,20 @@ func TestCrawl(t *testing.T) {
 		ID:          "2",
 		Updated:     nil,
 		Created:     &published,
-		Enclosure: &feeder.Enclosure{
-			URL:    "http://ogp.me/logo.png",
-			Type:   "image/png",
-			Length: "0",
-		}, Content: "",
-	}}}
+		Content:     "",
+	}}
 
-	fetcher1 := &mockFetcher{Id: "1"}
-	fetcher2 := &mockFetcher{Id: "2"}
-	items := feeder.Crawl(fetcher1, fetcher2)
+	crawler1 := &mockFetcher{Id: "1"}
+	crawler2 := &mockFetcher{Id: "2"}
+	items, err := feeder.Crawl(crawler1, crawler2)
+	if err != nil {
+		t.Errorf("Crawl() shoud return nil error error = %v", err)
+		return
+	}
 
-	if !reflect.DeepEqual(*expected, *items) {
-		diffs := pretty.Diff(*expected, *items)
+	if !reflect.DeepEqual(&expected, &items) {
+		diffs := pretty.Diff(expected, items)
 		t.Log(pretty.Println(diffs))
 		t.Error("Crawl does not match.")
-
 	}
 }

go.mod

@@ -11,4 +11,5 @@ require (
 	github.com/pkg/errors v0.8.1
 	github.com/stretchr/testify v1.3.0 // indirect
 	golang.org/x/net v0.0.0-20190311183353-d8887717615a // indirect
+	golang.org/x/sync v0.0.0-20190423024810-112230192c58
 )

go.sum

@@ -27,5 +27,7 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=