Skip to content

Commit

Permalink
WIP: Only fail if all packages fail to be polled for
Browse files Browse the repository at this point in the history
  • Loading branch information
tom--pollard committed Jun 7, 2021
1 parent 940c020 commit dcde494
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 14 deletions.
12 changes: 12 additions & 0 deletions feeds/feed.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package feeds

import (
"errors"
"fmt"
"time"
)

const schemaVer = "1.0"

var ErrNoPackagesPolled = errors.New("no packages were successfully polled")

type UnsupportedOptionError struct {
Option string
Feed string
Expand Down Expand Up @@ -37,6 +40,15 @@ type Package struct {
SchemaVer string `json:"schema_ver"`
}

type FailedPackagePoll struct {
Err error
Name string
}

func (err FailedPackagePoll) Error() string {
return fmt.Sprintf("Polling for package %s returned error: %v", err.Name, err.Err)
}

func NewPackage(created time.Time, name, version, feed string) *Package {
return &Package{
Name: name,
Expand Down
36 changes: 25 additions & 11 deletions feeds/pypi/pypi.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ 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"
Expand Down Expand Up @@ -97,7 +99,7 @@ func fetchPackages(baseURL string) ([]*Package, error) {
return rssResponse.Packages, nil
}

func fetchCriticalPackages(baseURL string, packageList []string) ([]*Package, error) {
func fetchCriticalPackages(baseURL string, packageList []string) ([]*Package, []error) {
responseChannel := make(chan *Response)
errChannel := make(chan error)

Expand All @@ -106,27 +108,27 @@ func fetchCriticalPackages(baseURL string, packageList []string) ([]*Package, er
packageDataPath := fmt.Sprintf(packagePathFormat, pkgName)
pkgURL, err := utils.URLPathJoin(baseURL, packageDataPath)
if err != nil {
errChannel <- err
errChannel <- feeds.FailedPackagePoll{Name: pkgName, Err: err}
return
}
resp, err := httpClient.Get(pkgURL)
if err != nil {
errChannel <- err
errChannel <- feeds.FailedPackagePoll{Name: pkgName, Err: err}
return
}
defer resp.Body.Close()

err = utils.CheckResponseStatus(resp)
if err != nil {
errChannel <- fmt.Errorf("failed to fetch pypi package data: %w", err)
errChannel <- feeds.FailedPackagePoll{Name: pkgName, Err: fmt.Errorf("failed to fetch pypi package data: %w", err)}
return
}

rssResponse := &Response{}
reader := utils.NewUTF8OnlyReader(resp.Body)
err = xml.NewDecoder(reader).Decode(rssResponse)
if err != nil {
errChannel <- err
errChannel <- feeds.FailedPackagePoll{Name: pkgName, Err: err}
return
}

Expand All @@ -135,15 +137,16 @@ func fetchCriticalPackages(baseURL string, packageList []string) ([]*Package, er
}

pkgs := []*Package{}
errs := []error{}
for i := 0; i < len(packageList); i++ {
select {
case response := <-responseChannel:
pkgs = append(pkgs, response.Packages...)
case err := <-errChannel:
return nil, err
errs = append(errs, err)
}
}
return pkgs, nil
return pkgs, errs
}

type Feed struct {
Expand All @@ -168,18 +171,29 @@ func (feed Feed) Latest(cutoff time.Time) ([]*feeds.Package, error) {
pkgs := []*feeds.Package{}
var pypiPackages []*Package
var err error
var errs []error

if feed.packages == nil {
// Firehose fetch all packages.
// If this fails then we need to return, as it's the only source of
// data.
pypiPackages, err = fetchPackages(feed.baseURL)
if err != nil {
return nil, err
}
} else {
// Fetch specific packages individually from configured packages list.
pypiPackages, err = fetchCriticalPackages(feed.baseURL, *feed.packages)
pypiPackages, errs = fetchCriticalPackages(feed.baseURL, *feed.packages)
for _, err := range errs {
// 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
}
}

if err != nil {
return nil, err
}
for _, pkg := range pypiPackages {
pkgName, err := pkg.Name()
if err != nil {
Expand Down
36 changes: 33 additions & 3 deletions feeds/pypi/pypi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/ossf/package-feeds/events"
"github.com/ossf/package-feeds/feeds"
"github.com/ossf/package-feeds/utils"
testutils "github.com/ossf/package-feeds/utils/test"
)

Expand Down Expand Up @@ -105,7 +104,7 @@ func TestPypiCriticalLatest(t *testing.T) {
}
}

func TestPypiNotFound(t *testing.T) {
func TestPypiAllNotFound(t *testing.T) {
t.Parallel()

handlers := map[string]testutils.HTTPHandlerFunc{
Expand All @@ -131,11 +130,42 @@ func TestPypiNotFound(t *testing.T) {
if err == nil {
t.Fatalf("feed.Latest() was successful when an error was expected")
}
if !errors.Is(err, utils.ErrUnsuccessfulRequest) {
if !errors.Is(err, feeds.ErrNoPackagesPolled) {
t.Fatalf("feed.Latest() returned an error which did not match the expected error")
}
}

func TestPypiPartialNotFound(t *testing.T) {
t.Parallel()

handlers := map[string]testutils.HTTPHandlerFunc{
"/rss/project/foopy/releases.xml": foopyReleasesResponse,
"/rss/project/barpy/releases.xml": testutils.NotFoundHandlerFunc,
}
packages := []string{
"foopy",
"barpy",
}
srv := testutils.HTTPServerMock(handlers)

feed, err := New(feeds.FeedOptions{
Packages: &packages,
}, events.NewNullHandler())
if err != nil {
t.Fatalf("Failed to create pypi feed: %v", err)
}
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")
}
if len(pkgs) != 2 {
t.Fatalf("Latest() produced %v packages instead of the expected %v", len(pkgs), 2)
}
}

// Mock data for pypi firehose with all packages.
func updatesXMLHandle(w http.ResponseWriter, r *http.Request) {
_, err := w.Write([]byte(`
Expand Down

0 comments on commit dcde494

Please sign in to comment.