From 74e67a9c966245c32626b6ae752f650e4ac8d082 Mon Sep 17 00:00:00 2001 From: Tom Pollard Date: Mon, 7 Jun 2021 15:55:31 +0100 Subject: [PATCH] WIP: Allow for per package error handling --- feeds/feed.go | 2 +- feeds/pypi/pypi.go | 22 +++++++++------------- feeds/pypi/pypi_test.go | 22 +++++++++++----------- feeds/scheduler/feed_group.go | 16 +++++++--------- feeds/scheduler/feed_group_test.go | 2 +- feeds/scheduler/mocks.go | 6 +++--- feeds/scheduler/scheduler.go | 2 +- 7 files changed, 33 insertions(+), 39 deletions(-) diff --git a/feeds/feed.go b/feeds/feed.go index e6715bca..3719b421 100644 --- a/feeds/feed.go +++ b/feeds/feed.go @@ -16,7 +16,7 @@ type UnsupportedOptionError struct { } type ScheduledFeed interface { - Latest(cutoff time.Time) ([]*Package, error) + Latest(cutoff time.Time) ([]*Package, []error) GetFeedOptions() FeedOptions GetName() string } diff --git a/feeds/pypi/pypi.go b/feeds/pypi/pypi.go index a0ded959..b43ad55f 100644 --- a/feeds/pypi/pypi.go +++ b/feeds/pypi/pypi.go @@ -8,8 +8,6 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" - "github.com/ossf/package-feeds/events" "github.com/ossf/package-feeds/feeds" "github.com/ossf/package-feeds/utils" @@ -167,11 +165,11 @@ func New(feedOptions feeds.FeedOptions, eventHandler *events.Handler) (*Feed, er }, nil } -func (feed Feed) Latest(cutoff time.Time) ([]*feeds.Package, error) { +func (feed Feed) Latest(cutoff time.Time) ([]*feeds.Package, []error) { pkgs := []*feeds.Package{} var pypiPackages []*Package - var err error var errs []error + var err error if feed.packages == nil { // Firehose fetch all packages. @@ -179,29 +177,27 @@ func (feed Feed) Latest(cutoff time.Time) ([]*feeds.Package, error) { // data. pypiPackages, err = fetchPackages(feed.baseURL) if err != nil { - return nil, err + return nil, append(errs, err) } } else { // Fetch specific packages individually from configured packages list. pypiPackages, errs = fetchCriticalPackages(feed.baseURL, *feed.packages) - for _, err := range errs { - // TODO: This could be an entry point for a 'lossy' type of event. - log.Errorf(err.Error()) - } if len(pypiPackages) == 0 { // If none of the packages were successfully polled for, return early. - return nil, feeds.ErrNoPackagesPolled + return nil, append(errs, feeds.ErrNoPackagesPolled) } } for _, pkg := range pypiPackages { pkgName, err := pkg.Name() if err != nil { - return nil, err + errs = append(errs, err) + continue } pkgVersion, err := pkg.Version() if err != nil { - return nil, err + errs = append(errs, err) + continue } pkg := feeds.NewPackage(pkg.CreatedDate.Time, pkgName, pkgVersion, FeedName) pkgs = append(pkgs, pkg) @@ -213,7 +209,7 @@ func (feed Feed) Latest(cutoff time.Time) ([]*feeds.Package, error) { } pkgs = feeds.ApplyCutoff(pkgs, cutoff) - return pkgs, nil + return pkgs, errs } func (feed Feed) GetPackageList() *[]string { diff --git a/feeds/pypi/pypi_test.go b/feeds/pypi/pypi_test.go index 5b393a10..1bbac8c5 100644 --- a/feeds/pypi/pypi_test.go +++ b/feeds/pypi/pypi_test.go @@ -26,8 +26,8 @@ func TestPypiLatest(t *testing.T) { feed.baseURL = srv.URL cutoff := time.Date(1970, 1, 1, 0, 0, 0, 0, time.UTC) - pkgs, err := feed.Latest(cutoff) - if err != nil { + pkgs, errs := feed.Latest(cutoff) + if len(errs) != 0 { t.Fatalf("feed.Latest returned error: %v", err) } @@ -73,8 +73,8 @@ func TestPypiCriticalLatest(t *testing.T) { feed.baseURL = srv.URL cutoff := time.Date(1970, 1, 1, 0, 0, 0, 0, time.UTC) - pkgs, err := feed.Latest(cutoff) - if err != nil { + pkgs, errs := feed.Latest(cutoff) + if len(errs) != 0 { t.Fatalf("Failed to call Latest() with err: %v", err) } @@ -126,11 +126,11 @@ func TestPypiAllNotFound(t *testing.T) { feed.baseURL = srv.URL cutoff := time.Date(1970, 1, 1, 0, 0, 0, 0, time.UTC) - _, err = feed.Latest(cutoff) - if err == nil { - t.Fatalf("feed.Latest() was successful when an error was expected") + _, errs := feed.Latest(cutoff) + if len(errs) == 0 { + t.Fatalf("feed.Latest() was successful when errors were expected") } - if !errors.Is(err, feeds.ErrNoPackagesPolled) { + if !errors.Is(errs[len(errs)-1], feeds.ErrNoPackagesPolled) { t.Fatalf("feed.Latest() returned an error which did not match the expected error") } } @@ -157,9 +157,9 @@ func TestPypiPartialNotFound(t *testing.T) { feed.baseURL = srv.URL cutoff := time.Date(1970, 1, 1, 0, 0, 0, 0, time.UTC) - pkgs, err := feed.Latest(cutoff) - if err != nil { - t.Fatalf("feed.Latest() was unsuccessful when an error wasn't expected") + pkgs, errs := feed.Latest(cutoff) + if len(errs) != 1 { + t.Fatalf("feed.Latest() returned no errors when an error was expected") } if len(pkgs) != 2 { t.Fatalf("Latest() produced %v packages instead of the expected %v", len(pkgs), 2) diff --git a/feeds/scheduler/feed_group.go b/feeds/scheduler/feed_group.go index 1d817f9a..bac2d817 100644 --- a/feeds/scheduler/feed_group.go +++ b/feeds/scheduler/feed_group.go @@ -38,10 +38,9 @@ func (fg *FeedGroup) Run() { } func (fg *FeedGroup) PollAndPublish() (int, error) { - pkgs, errs := fg.poll() - if len(errs) > 0 { - return 0, errs[0] - } else if len(pkgs) == 0 { + // poll() logs errs, so no need to handle. + pkgs, _ := fg.poll() + if len(pkgs) == 0 { return 0, nil } @@ -64,7 +63,7 @@ func (fg *FeedGroup) poll() ([]*feeds.Package, []error) { name: feed.GetName(), feed: feed, } - result.packages, result.err = feed.Latest(fg.lastPoll) + result.packages, result.errs = feed.Latest(fg.lastPoll) results <- result }(feed) } @@ -74,10 +73,9 @@ func (fg *FeedGroup) poll() ([]*feeds.Package, []error) { result := <-results logger := log.WithField("feed", result.name) - if result.err != nil { - logger.WithError(result.err).Error("Error fetching packages") - errs = append(errs, result.err) - continue + for _, err := range result.errs { + logger.WithError(err).Error("Error fetching packages") + errs = append(errs, err) } for _, pkg := range result.packages { log.WithFields(log.Fields{ diff --git a/feeds/scheduler/feed_group_test.go b/feeds/scheduler/feed_group_test.go index b863cabd..039e071e 100644 --- a/feeds/scheduler/feed_group_test.go +++ b/feeds/scheduler/feed_group_test.go @@ -54,7 +54,7 @@ func TestFeedGroupPollWithErr(t *testing.T) { mockFeeds := []feeds.ScheduledFeed{ mockFeed{ - err: errPackage, + errs: []error{errPackage}, }, mockFeed{ packages: []*feeds.Package{ diff --git a/feeds/scheduler/mocks.go b/feeds/scheduler/mocks.go index 0b05eb30..aada2bb1 100644 --- a/feeds/scheduler/mocks.go +++ b/feeds/scheduler/mocks.go @@ -9,7 +9,7 @@ import ( type mockFeed struct { packages []*feeds.Package - err error + errs []error options feeds.FeedOptions } @@ -21,8 +21,8 @@ func (feed mockFeed) GetFeedOptions() feeds.FeedOptions { return feed.options } -func (feed mockFeed) Latest(cutoff time.Time) ([]*feeds.Package, error) { - return feed.packages, feed.err +func (feed mockFeed) Latest(cutoff time.Time) ([]*feeds.Package, []error) { + return feed.packages, feed.errs } type mockPublisher struct { diff --git a/feeds/scheduler/scheduler.go b/feeds/scheduler/scheduler.go index 51454829..993734ce 100644 --- a/feeds/scheduler/scheduler.go +++ b/feeds/scheduler/scheduler.go @@ -33,7 +33,7 @@ type pollResult struct { name string feed feeds.ScheduledFeed packages []*feeds.Package - err error + errs []error } // Runs several services for the operation of scheduler, this call is blocking until application exit