provide context in errors

Author: dmitriy kalinin

patch/op_definition.go

@@ -1,37 +1,45 @@
 package patch
 
 import (
+	"encoding/json"
 	"fmt"
+	"strings"
 )
 
 // OpDefinition struct is useful for JSON and YAML unmarshaling
 type OpDefinition struct {
-	Type  string
-	Path  *string
-	Value *interface{}
+	Type  string       `json:",omitempty"`
+	Path  *string      `json:",omitempty"`
+	Value *interface{} `json:",omitempty"`
 }
 
+type parser struct{}
+
 func NewOpsFromDefinitions(opDefs []OpDefinition) (Ops, error) {
 	var ops []Op
-	var op Op
-	var err error
+	var p parser
 
 	for i, opDef := range opDefs {
+		var op Op
+		var err error
+
+		opFmt := p.fmtOpDef(opDef)
+
 		switch opDef.Type {
 		case "replace":
-			op, err = newReplaceOp(opDef)
+			op, err = p.newReplaceOp(opDef)
 			if err != nil {
-				return nil, fmt.Errorf("Replace operation [%d]: %s", i, err)
+				return nil, fmt.Errorf("Replace operation [%d]: %s within\n%s", i, err, opFmt)
 			}
 
 		case "remove":
-			op, err = newRemoveOp(opDef)
+			op, err = p.newRemoveOp(opDef)
 			if err != nil {
-				return nil, fmt.Errorf("Remove operation [%d]: %s", i, err)
+				return nil, fmt.Errorf("Remove operation [%d]: %s within\n%s", i, err, opFmt)
 			}
 
 		default:
-			return nil, fmt.Errorf("Unknown operation [%d] with type '%s'", i, opDef.Type)
+			return nil, fmt.Errorf("Unknown operation [%d] with type '%s' within\n%s", i, opDef.Type, opFmt)
 		}
 
 		ops = append(ops, op)
@@ -40,7 +48,7 @@ func NewOpsFromDefinitions(opDefs []OpDefinition) (Ops, error) {
 	return Ops(ops), nil
 }
 
-func newReplaceOp(opDef OpDefinition) (ReplaceOp, error) {
+func (parser) newReplaceOp(opDef OpDefinition) (ReplaceOp, error) {
 	if opDef.Path == nil {
 		return ReplaceOp{}, fmt.Errorf("Missing path")
 	}
@@ -57,7 +65,7 @@ func newReplaceOp(opDef OpDefinition) (ReplaceOp, error) {
 	return ReplaceOp{Path: ptr, Value: *opDef.Value}, nil
 }
 
-func newRemoveOp(opDef OpDefinition) (RemoveOp, error) {
+func (parser) newRemoveOp(opDef OpDefinition) (RemoveOp, error) {
 	if opDef.Path == nil {
 		return RemoveOp{}, fmt.Errorf("Missing path")
 	}
@@ -73,3 +81,22 @@ func newRemoveOp(opDef OpDefinition) (RemoveOp, error) {
 
 	return RemoveOp{Path: ptr}, nil
 }
+
+func (parser) fmtOpDef(opDef OpDefinition) string {
+	var (
+		redactedVal interface{} = "<redacted>"
+		htmlDecoder             = strings.NewReplacer("\\u003c", "<", "\\u003e", ">")
+	)
+
+	if opDef.Value != nil {
+		// can't JSON serialize generic interface{} anyway
+		opDef.Value = &redactedVal
+	}
+
+	bytes, err := json.MarshalIndent(opDef, "", "  ")
+	if err != nil {
+		return "<unknown>"
+	}
+
+	return htmlDecoder.Replace(string(bytes))
+}

patch/op_definition_test.go

@@ -12,6 +12,7 @@ var _ = Describe("NewOpsFromDefinitions", func() {
 		path                    = "/abc"
 		invalidPath             = "abc"
 		val         interface{} = 123
+		complexVal  interface{} = map[interface{}]interface{}{123: 123}
 	)
 
 	It("supports 'replace' and 'remove' operations", func() {
@@ -32,52 +33,90 @@ var _ = Describe("NewOpsFromDefinitions", func() {
 	It("returns error if operation type is unknown", func() {
 		_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "test"}})
 		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(Equal("Unknown operation [0] with type 'test'"))
+		Expect(err.Error()).To(Equal(`Unknown operation [0] with type 'test' within
+{
+  "Type": "test"
+}`))
 	})
 
 	It("returns error if operation type is find since it's not useful in list of operations", func() {
 		_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "find"}})
 		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(Equal("Unknown operation [0] with type 'find'"))
+		Expect(err.Error()).To(ContainSubstring("Unknown operation [0] with type 'find'"))
+	})
+
+	It("allows values to be complex in error messages", func() {
+		_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "test", Path: &invalidPath, Value: &complexVal}})
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(Equal(`Unknown operation [0] with type 'test' within
+{
+  "Type": "test",
+  "Path": "abc",
+  "Value": "<redacted>"
+}`))
 	})
 
 	Describe("replace", func() {
 		It("requires path", func() {
 			_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "replace"}})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("Replace operation [0]: Missing path"))
+			Expect(err.Error()).To(Equal(`Replace operation [0]: Missing path within
+{
+  "Type": "replace"
+}`))
 		})
 
 		It("requires value", func() {
 			_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "replace", Path: &path}})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("Replace operation [0]: Missing value"))
+			Expect(err.Error()).To(Equal(`Replace operation [0]: Missing value within
+{
+  "Type": "replace",
+  "Path": "/abc"
+}`))
 		})
 
 		It("requires valid path", func() {
 			_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "replace", Path: &invalidPath, Value: &val}})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("Replace operation [0]: Invalid path: Expected to start with '/'"))
+			Expect(err.Error()).To(Equal(`Replace operation [0]: Invalid path: Expected to start with '/' within
+{
+  "Type": "replace",
+  "Path": "abc",
+  "Value": "<redacted>"
+}`))
 		})
 	})
 
 	Describe("remove", func() {
 		It("requires path", func() {
 			_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "remove"}})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("Remove operation [0]: Missing path"))
+			Expect(err.Error()).To(Equal(`Remove operation [0]: Missing path within
+{
+  "Type": "remove"
+}`))
 		})
 
 		It("does not allow value", func() {
 			_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "remove", Path: &path, Value: &val}})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("Remove operation [0]: Cannot specify value"))
+			Expect(err.Error()).To(Equal(`Remove operation [0]: Cannot specify value within
+{
+  "Type": "remove",
+  "Path": "/abc",
+  "Value": "<redacted>"
+}`))
 		})
 
 		It("requires valid path", func() {
 			_, err := NewOpsFromDefinitions([]OpDefinition{{Type: "remove", Path: &invalidPath}})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("Remove operation [0]: Invalid path: Expected to start with '/'"))
+			Expect(err.Error()).To(Equal(`Remove operation [0]: Invalid path: Expected to start with '/' within
+{
+  "Type": "remove",
+  "Path": "abc"
+}`))
 		})
 	})
 })