From d4d7ae1ccfb71876b97ce1b31b8a764416ce5e4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Mon, 10 Sep 2018 21:02:18 +0200 Subject: [PATCH] tpl/data: Revise error handling in getJSON and getCSV The most important part being: Log ERROR, but do not stop the build on remote errors. Fixes #5076 --- tpl/data/data.go | 36 +++++++++++++++++++++++------------- tpl/data/data_test.go | 14 +++++++++----- tpl/data/resources_test.go | 12 +++++++++--- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/tpl/data/data.go b/tpl/data/data.go index 0dd2b26255a..0f72ed8f75e 100644 --- a/tpl/data/data.go +++ b/tpl/data/data.go @@ -18,6 +18,7 @@ import ( "encoding/csv" "encoding/json" "errors" + "fmt" "net/http" "strings" "time" @@ -50,7 +51,7 @@ func (ns *Namespace) GetCSV(sep string, urlParts ...string) (d [][]string, err e url := strings.Join(urlParts, "") var clearCacheSleep = func(i int, u string) { - jww.ERROR.Printf("Retry #%d for %s and sleeping for %s", i, url, resSleep) + ns.deps.Log.WARN.Printf("Retry #%d for %s and sleeping for %s", i, url, resSleep) time.Sleep(resSleep) deleteCache(url, ns.deps.Fs.Source, ns.deps.Cfg) } @@ -59,8 +60,7 @@ func (ns *Namespace) GetCSV(sep string, urlParts ...string) (d [][]string, err e var req *http.Request req, err = http.NewRequest("GET", url, nil) if err != nil { - jww.ERROR.Printf("Failed to create request for getCSV: %s", err) - return nil, err + return nil, fmt.Errorf("Failed to create request for getCSV: %s", err) } req.Header.Add("Accept", "text/csv") @@ -69,22 +69,28 @@ func (ns *Namespace) GetCSV(sep string, urlParts ...string) (d [][]string, err e var c []byte c, err = ns.getResource(req) if err != nil { - jww.ERROR.Printf("Failed to read csv resource %q with error message %s", url, err) - return nil, err + ns.deps.Log.ERROR.Printf("Failed to read CSV resource %q: %s", url, err) + return nil, nil } if !bytes.Contains(c, []byte(sep)) { - err = errors.New("Cannot find separator " + sep + " in CSV.") - return + ns.deps.Log.ERROR.Printf("Cannot find separator %s in CSV for %s", sep, url) + return nil, nil } if d, err = parseCSV(c, sep); err != nil { - jww.ERROR.Printf("Failed to parse csv file %s with error message %s", url, err) + ns.deps.Log.WARN.Printf("Failed to parse CSV file %s: %s", url, err) clearCacheSleep(i, url) continue } break } + + if err != nil { + ns.deps.Log.ERROR.Printf("Failed to read CSV resource %q: %s", url, err) + return nil, nil + } + return } @@ -98,8 +104,7 @@ func (ns *Namespace) GetJSON(urlParts ...string) (v interface{}, err error) { var req *http.Request req, err = http.NewRequest("GET", url, nil) if err != nil { - jww.ERROR.Printf("Failed to create request for getJSON: %s", err) - return nil, err + return nil, fmt.Errorf("Failed to create request for getJSON: %s", err) } req.Header.Add("Accept", "application/json") @@ -107,20 +112,25 @@ func (ns *Namespace) GetJSON(urlParts ...string) (v interface{}, err error) { var c []byte c, err = ns.getResource(req) if err != nil { - jww.ERROR.Printf("Failed to get json resource %s with error message %s", url, err) + jww.ERROR.Printf("Failed to get JSON resource %s: %s", url, err) return nil, err } err = json.Unmarshal(c, &v) if err != nil { - jww.ERROR.Printf("Cannot read json from resource %s with error message %s", url, err) - jww.ERROR.Printf("Retry #%d for %s and sleeping for %s", i, url, resSleep) + ns.deps.Log.WARN.Printf("Cannot read JSON from resource %s: %s", url, err) + ns.deps.Log.WARN.Printf("Retry #%d for %s and sleeping for %s", i, url, resSleep) time.Sleep(resSleep) deleteCache(url, ns.deps.Fs.Source, ns.deps.Cfg) continue } break } + + if err != nil { + jww.ERROR.Printf("Failed to get JSON resource %s: %s", url, err) + return nil, err + } return } diff --git a/tpl/data/data_test.go b/tpl/data/data_test.go index 9b21dc8aaff..0396e85a1fa 100644 --- a/tpl/data/data_test.go +++ b/tpl/data/data_test.go @@ -21,14 +21,13 @@ import ( "strings" "testing" + jww "github.com/spf13/jwalterweatherman" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestGetCSV(t *testing.T) { - t.Parallel() - - ns := newTestNs() for i, test := range []struct { sep string @@ -78,6 +77,8 @@ func TestGetCSV(t *testing.T) { } { msg := fmt.Sprintf("Test %d", i) + ns := newTestNs() + // Setup HTTP test server var srv *httptest.Server srv, ns.client = getTestServer(func(w http.ResponseWriter, r *http.Request) { @@ -108,11 +109,14 @@ func TestGetCSV(t *testing.T) { // Get on with it got, err := ns.GetCSV(test.sep, test.url) + require.NoError(t, err, msg) + if _, ok := test.expect.(bool); ok { - assert.Error(t, err, msg) + assert.Equal(t, 1, int(ns.deps.Log.LogCountForLevelsGreaterThanorEqualTo(jww.LevelError))) + assert.Nil(t, got) continue } - require.NoError(t, err, msg) + assert.Equal(t, 0, int(ns.deps.Log.LogCountForLevelsGreaterThanorEqualTo(jww.LevelError))) require.NotNil(t, got, msg) assert.EqualValues(t, test.expect, got, msg) diff --git a/tpl/data/resources_test.go b/tpl/data/resources_test.go index f6baae18b65..c1da36d055b 100644 --- a/tpl/data/resources_test.go +++ b/tpl/data/resources_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + "github.com/gohugoio/hugo/common/loggers" "github.com/gohugoio/hugo/config" "github.com/gohugoio/hugo/deps" "github.com/gohugoio/hugo/helpers" @@ -171,10 +172,15 @@ func newDeps(cfg config.Provider) *deps.Deps { if err != nil { panic(err) } + + logger := loggers.NewErrorLogger() + return &deps.Deps{ - Cfg: cfg, - Fs: hugofs.NewMem(l), - ContentSpec: cs, + Cfg: cfg, + Fs: hugofs.NewMem(l), + ContentSpec: cs, + Log: logger, + DistinctErrorLog: helpers.NewDistinctLogger(logger.ERROR), } }