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

diff --git a/tpl/data/data.go b/tpl/data/data.go
index 0dd2b26255a277e5e3ac44b293b6752fb3021e43..14a4975a5d96613624afd6715d10076bbff7c44a 100644 (file)
--- a/tpl/data/data.go
+++ b/tpl/data/data.go
@@ -18,12 +18,12 @@ import (
        "encoding/csv"
        "encoding/json"
        "errors"
+       "fmt"
        "net/http"
        "strings"
        "time"
 
        "github.com/gohugoio/hugo/deps"
-       jww "github.com/spf13/jwalterweatherman"
 )
 
 // New returns a new instance of the data-namespaced template functions.
@@ -50,7 +50,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 +59,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 for resource %s: %s", url, err)
                }
 
                req.Header.Add("Accept", "text/csv")
@@ -69,22 +68,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 +103,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 resource %s: %s", url, err)
                }
 
                req.Header.Add("Accept", "application/json")
@@ -107,20 +111,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)
-                       return nil, err
+                       ns.deps.Log.ERROR.Printf("Failed to get JSON resource %s: %s", url, err)
+                       return nil, nil
                }
 
                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 {
+               ns.deps.Log.ERROR.Printf("Failed to get JSON resource %s: %s", url, err)
+               return nil, nil
+       }
        return
 }
 

diff --git a/tpl/data/data_test.go b/tpl/data/data_test.go
index 9b21dc8aaff987a0a30fdfa196e114c03c17f9eb..6bee0d524812dfc409a1758e26b7b3ce7d84c22e 100644 (file)
--- a/tpl/data/data_test.go
+++ b/tpl/data/data_test.go
@@ -21,6 +21,8 @@ import (
        "strings"
        "testing"
 
+       jww "github.com/spf13/jwalterweatherman"
+
        "github.com/stretchr/testify/assert"
        "github.com/stretchr/testify/require"
 )
@@ -28,8 +30,6 @@ import (
 func TestGetCSV(t *testing.T) {
        t.Parallel()
 
-       ns := newTestNs()
-
        for i, test := range []struct {
                sep     string
                url     string
@@ -78,6 +78,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 +110,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)
+                       require.Equal(t, 1, int(ns.deps.Log.LogCountForLevelsGreaterThanorEqualTo(jww.LevelError)))
+                       require.Nil(t, got)
                        continue
                }
-               require.NoError(t, err, msg)
+               require.Equal(t, 0, int(ns.deps.Log.LogCountForLevelsGreaterThanorEqualTo(jww.LevelError)))
                require.NotNil(t, got, msg)
 
                assert.EqualValues(t, test.expect, got, msg)
@@ -122,8 +127,6 @@ func TestGetCSV(t *testing.T) {
 func TestGetJSON(t *testing.T) {
        t.Parallel()
 
-       ns := newTestNs()
-
        for i, test := range []struct {
                url     string
                content string
@@ -137,12 +140,12 @@ func TestGetJSON(t *testing.T) {
                {
                        `http://malformed/`,
                        `{gomeetup:["Sydney","San Francisco","Stockholm"]}`,
-                       false,
+                       jww.LevelError,
                },
                {
                        `http://nofound/404`,
                        ``,
-                       false,
+                       jww.LevelError,
                },
                // Locals
                {
@@ -153,10 +156,12 @@ func TestGetJSON(t *testing.T) {
                {
                        "fail/no-file",
                        "",
-                       false,
+                       jww.LevelError,
                },
        } {
+
                msg := fmt.Sprintf("Test %d", i)
+               ns := newTestNs()
 
                // Setup HTTP test server
                var srv *httptest.Server
@@ -189,10 +194,18 @@ func TestGetJSON(t *testing.T) {
                got, err := ns.GetJSON(test.url)
 
                if _, ok := test.expect.(bool); ok {
-                       assert.Error(t, err, msg)
+                       require.Error(t, err, msg)
+                       continue
+               }
+
+               if errLevel, ok := test.expect.(jww.Threshold); ok {
+                       logCount := ns.deps.Log.LogCountForLevelsGreaterThanorEqualTo(errLevel)
+                       require.True(t, logCount >= 1, fmt.Sprintf("got log count %d", logCount))
                        continue
                }
                require.NoError(t, err, msg)
+
+               require.Equal(t, 0, int(ns.deps.Log.LogCountForLevelsGreaterThanorEqualTo(jww.LevelError)), msg)
                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 f6baae18b65fa43aeca6a59c2d334ede1ccc5c88..c1da36d055b8c5747326998b8242a0cdcf4a18e0 100644 (file)
--- 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),
        }
 }