encoding/json: SyntaxError does not have "json:" prefix

[kevinburke1]

It was my understanding that most errors created inside of a standard library package should be prefixed with the package name. For example, the error message for sql.ErrNoRows is errors.New("sql: no rows in result set"), not errors.New("no rows in result set").

Some errors in the json package are returned without a leading prefix. I noticed this because I hit one and it took some digging to figure out where in the codebase the error was actually coming from.

These tests in scanner_test.go check that the error matches a SyntaxError without a leading prefix.

var indentErrorTests = []indentErrorTest{
	{`{"X": "foo", "Y"}`, &SyntaxError{"invalid character '}' after object key", 17}},
	{`{"X": "foo" "Y": "bar"}`, &SyntaxError{"invalid character '\"' after object key:value pair", 13}},
}

func TestIndentErrors(t *testing.T) {
	for i, tt := range indentErrorTests {
		slice := make([]uint8, 0)
		buf := bytes.NewBuffer(slice)
		if err := Indent(buf, []uint8(tt.in), "", ""); err != nil {
			if !reflect.DeepEqual(err, tt.err) {
				t.Errorf("#%d: Indent: %#v", i, err)
				continue
			}
		}
	}
}

Is that right? Shouldn't those error messages have a leading "json:" prefix?

[kevinburke1]

I can submit a fix, just let me know if that's something it would be good to fix

[mvdan]

I assume you mean that their Error() string methods should begin with json: . I'd probably agree there, though I haven't taken a closer look at why the errors in the package aren't stringified in a consistent way.

I'm not sure that the prefix should be part of the error value itself, though. If the error type is json.SyntaxError, having its message also contain json: seems a bit redundant. The fmt.Errorf errors do contain the prefix, but that's just because they are generic errors.

[dpinela]

I've been thrown off by this before.

[gopherbot]

Change https://golang.org/cl/263619 mentions this issue: encoding/json: add "json: " prefix to SyntaxError messages

[mvdan]

I'm still not sure about this. Is there a precedent with other exported error types in the standard library whose Error method uses a prefix? It feels redundant for a json error to prepend its own package name. I also can't recall any other standard library error behaving this way, besides non-typed fmt.Errorf("pkgname: message") errors.

For example, the os errors use the operation or syscall as the prefix, so you get open: ... not os: open: ....

[kevinburke1]

Every other named error in that package prepends the prefix:

• UnmarshalTypeError
• UnmarshalFieldError
• InvalidUnmarshalError
• InvalidUTF8Error
• MarshalerError
• UnsupportedTypeError
• UnsupportedValueError

SyntaxError is the only one that doesn't.

I wasn't aware that this wasn't common practice across the standard library, if it isn't then maybe we should leave it as is.

[mvdan]

Ah, you're right, there's plenty of precedent in the same package. I wonder if we would break any existing valid programs, but I would imagine not. I'll go review.

[gopherbot]

Change https://golang.org/cl/273747 mentions this issue: encoding/json: revert "add "json: " prefix to SyntaxError messages"

[dsnet]

Re-opening. I still think this is worth doing in the future, but we may want to batch it with other small changes that may technically break users who depend on unspecified behavior. See #42675