Include positions filename in the error when YAML unmarshal fails. (#…

…1311)

* Include positions filename in the error when YAML unmarshal fails.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Include filename when parsing of YAML/JSON file fails.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

pkg/cfg/cfg.go

@@ -29,7 +29,7 @@ func Unmarshal(dst interface{}, sources ...Source) error {
 
 	for _, source := range sources {
 		if err := source(dst); err != nil {
-			return errors.Wrap(err, "sourcing")
+			return err
 		}
 	}
 	return nil

pkg/cfg/files.go

@@ -5,7 +5,8 @@ import (
 	"flag"
 	"io/ioutil"
 
-	yaml "gopkg.in/yaml.v2"
+	"github.com/pkg/errors"
+	"gopkg.in/yaml.v2"
 )
 
 // JSON returns a Source that opens the supplied `.json` file and loads it.
@@ -20,7 +21,8 @@ func JSON(f *string) Source {
 			return err
 		}
 
-		return dJSON(j)(dst)
+		err = dJSON(j)(dst)
+		return errors.Wrap(err, *f)
 	}
 }
 
@@ -43,7 +45,8 @@ func YAML(f *string) Source {
 			return err
 		}
 
-		return dYAML(y)(dst)
+		err = dYAML(y)(dst)
+		return errors.Wrap(err, *f)
 	}
 }
 

pkg/promtail/positions/positions.go

@@ -2,6 +2,7 @@ package positions
 
 import (
 	"flag"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -181,7 +182,8 @@ func (p *Positions) cleanup() {
 }
 
 func readPositionsFile(filename string) (map[string]string, error) {
-	buf, err := ioutil.ReadFile(filepath.Clean(filename))
+	cleanfn := filepath.Clean(filename)
+	buf, err := ioutil.ReadFile(cleanfn)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return map[string]string{}, nil
@@ -191,7 +193,7 @@ func readPositionsFile(filename string) (map[string]string, error) {
 
 	var p File
 	if err := yaml.UnmarshalStrict(buf, &p); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: %v", cleanfn, err)
 	}
 
 	return p.Positions, nil

pkg/promtail/positions/positions_test.go

@@ -0,0 +1,85 @@
+package positions
+
+import (
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func tempFilename(t *testing.T) string {
+	t.Helper()
+
+	temp, err := ioutil.TempFile("", "positions")
+	if err != nil {
+		t.Fatal("tempFilename:", err)
+	}
+	err = temp.Close()
+	if err != nil {
+		t.Fatal("tempFilename:", err)
+	}
+
+	name := temp.Name()
+	err = os.Remove(name)
+	if err != nil {
+		t.Fatal("tempFilename:", err)
+	}
+
+	return name
+}
+
+func TestReadPositionsOK(t *testing.T) {
+	temp := tempFilename(t)
+	defer func() {
+		_ = os.Remove(temp)
+	}()
+
+	yaml := []byte(`positions:
+  /tmp/random.log: "17623"
+`)
+	err := ioutil.WriteFile(temp, yaml, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	pos, err := readPositionsFile(temp)
+	require.NoError(t, err)
+	require.Equal(t, "17623", pos["/tmp/random.log"])
+}
+
+func TestReadPositionsFromDir(t *testing.T) {
+	temp := tempFilename(t)
+	err := os.Mkdir(temp, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		_ = os.Remove(temp)
+	}()
+
+	_, err = readPositionsFile(temp)
+	require.Error(t, err)
+	require.True(t, strings.Contains(err.Error(), temp)) // error must contain filename
+}
+
+func TestReadPositionsFromBadYaml(t *testing.T) {
+	temp := tempFilename(t)
+	defer func() {
+		_ = os.Remove(temp)
+	}()
+
+	badYaml := []byte(`positions:
+  /tmp/random.log: "176
+`)
+	err := ioutil.WriteFile(temp, badYaml, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = readPositionsFile(temp)
+	require.Error(t, err)
+	require.True(t, strings.Contains(err.Error(), temp)) // error must contain filename
+}