Skip to content

Commit

Permalink
Revert error message changes
Browse files Browse the repository at this point in the history
Changes in PR 65 modified error messages. While some of the changes read better,
also produced duplicate prefixes (like "error converting YAML to JSON: error converting YAML to JSON:").

Even the improvements broke downstream unit tests relying on exact prefix matching.

Reverting the error message changes before tagging a new release to avoid disrupting consumers
until we consider the impact of these changes more thoroughly.
  • Loading branch information
liggitt committed Oct 23, 2023
1 parent 65d71bb commit 3f79f32
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 6 deletions.
130 changes: 130 additions & 0 deletions err_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package yaml

import (
"strings"
"testing"
)

func TestErrors(t *testing.T) {
type Into struct {
Map map[string]interface{} `json:"map"`
Int int32 `json:"int"`
}

testcases := []struct {
Name string
Data string
UnmarshalPrefix string
UnmarshalStrictPrefix string
YAMLToJSONPrefix string
YAMLToJSONStrictPrefix string
}{
{
Name: "unmarshal syntax",
Data: `map: {`,
UnmarshalPrefix: `error converting YAML to JSON: yaml: line 1: `,
UnmarshalStrictPrefix: `error converting YAML to JSON: yaml: line 1: `,
YAMLToJSONPrefix: `yaml: line 1: `,
YAMLToJSONStrictPrefix: `yaml: line 1: `,
},
{
Name: "unmarshal type",
Data: `map: ""`,
UnmarshalPrefix: `error unmarshaling JSON: while decoding JSON: json: `,
UnmarshalStrictPrefix: `error unmarshaling JSON: while decoding JSON: json: `,
YAMLToJSONPrefix: ``,
YAMLToJSONStrictPrefix: ``,
},
{
Name: "unmarshal unknown",
Data: `unknown: {}`,
UnmarshalPrefix: ``,
UnmarshalStrictPrefix: `error unmarshaling JSON: while decoding JSON: json: `,
YAMLToJSONPrefix: ``,
YAMLToJSONStrictPrefix: ``,
},
{
Name: "unmarshal duplicate",
Data: `
int: 0
int: 0`,
UnmarshalPrefix: ``,
UnmarshalStrictPrefix: `error converting YAML to JSON: yaml: `,
YAMLToJSONPrefix: ``,
YAMLToJSONStrictPrefix: `yaml: unmarshal errors:`,
},
}

for _, tc := range testcases {
t.Run(tc.Name, func(t *testing.T) {
v := Into{}
if err := Unmarshal([]byte(tc.Data), &v); err == nil {
if len(tc.UnmarshalPrefix) > 0 {
t.Fatal("expected err")
}
} else {
if len(tc.UnmarshalPrefix) == 0 {
t.Fatalf("unexpected err %v", err)
}
if !strings.HasPrefix(err.Error(), tc.UnmarshalPrefix) {
t.Fatalf("expected '%s' to start with '%s'", err.Error(), tc.UnmarshalPrefix)
}
}

if err := UnmarshalStrict([]byte(tc.Data), &v); err == nil {
if len(tc.UnmarshalStrictPrefix) > 0 {
t.Fatal("expected err")
}
} else {
if len(tc.UnmarshalStrictPrefix) == 0 {
t.Fatalf("unexpected err %v", err)
}
if !strings.HasPrefix(err.Error(), tc.UnmarshalStrictPrefix) {
t.Fatalf("expected '%s' to start with '%s'", err.Error(), tc.UnmarshalStrictPrefix)
}
}

if _, err := YAMLToJSON([]byte(tc.Data)); err == nil {
if len(tc.YAMLToJSONPrefix) > 0 {
t.Fatal("expected err")
}
} else {
if len(tc.YAMLToJSONPrefix) == 0 {
t.Fatalf("unexpected err %v", err)
}
if !strings.HasPrefix(err.Error(), tc.YAMLToJSONPrefix) {
t.Fatalf("expected '%s' to start with '%s'", err.Error(), tc.YAMLToJSONPrefix)
}
}

if _, err := YAMLToJSONStrict([]byte(tc.Data)); err == nil {
if len(tc.YAMLToJSONStrictPrefix) > 0 {
t.Fatal("expected err")
}
} else {
if len(tc.YAMLToJSONStrictPrefix) == 0 {
t.Fatalf("unexpected err %v", err)
}
if !strings.HasPrefix(err.Error(), tc.YAMLToJSONStrictPrefix) {
t.Fatalf("expected '%s' to start with '%s'", err.Error(), tc.YAMLToJSONStrictPrefix)
}
}
})
}
}
15 changes: 9 additions & 6 deletions yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ func jsonUnmarshal(reader io.Reader, obj interface{}, opts ...JSONOpt) error {
for _, opt := range opts {
d = opt(d)
}
return d.Decode(obj)
if err := d.Decode(&obj); err != nil {
return fmt.Errorf("while decoding JSON: %v", err)
}
return nil
}

// JSONToYAML converts JSON to YAML. Notable implementation details:
Expand All @@ -110,13 +113,13 @@ func JSONToYAML(j []byte) ([]byte, error) {
// number type, so we can preserve number type throughout this process.
err := yaml.Unmarshal(j, &jsonObj)
if err != nil {
return nil, fmt.Errorf("error converting JSON to YAML: %w", err)
return nil, err
}

// Marshal this object into YAML.
yamlBytes, err := yaml.Marshal(jsonObj)
if err != nil {
return nil, fmt.Errorf("error converting JSON to YAML: %w", err)
return nil, err
}

return yamlBytes, nil
Expand Down Expand Up @@ -155,7 +158,7 @@ func yamlToJSONTarget(yamlBytes []byte, jsonTarget *reflect.Value, unmarshalFn f
var yamlObj interface{}
err := unmarshalFn(yamlBytes, &yamlObj)
if err != nil {
return nil, fmt.Errorf("error converting YAML to JSON: %w", err)
return nil, err
}

// YAML objects are not completely compatible with JSON objects (e.g. you
Expand All @@ -164,13 +167,13 @@ func yamlToJSONTarget(yamlBytes []byte, jsonTarget *reflect.Value, unmarshalFn f
// incompatibilties happen along the way.
jsonObj, err := convertToJSONableObject(yamlObj, jsonTarget)
if err != nil {
return nil, fmt.Errorf("error converting YAML to JSON: %w", err)
return nil, err
}

// Convert this object to JSON and return the data.
jsonBytes, err := json.Marshal(jsonObj)
if err != nil {
return nil, fmt.Errorf("error converting YAML to JSON: %w", err)
return nil, err
}
return jsonBytes, nil
}
Expand Down

0 comments on commit 3f79f32

Please sign in to comment.