code review 6938045: encoding/json: fix type assertion in error path

encoding/json: fix type assertion in error path

Update issue 4474.
Fixes issue 4222.

Not all code calls panic(error), some code calls panic(string)

[dave_cheney.net, 2012-12-15 11:16:46]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[remyoudompheng, 2012-12-15 11:48:35]

I don't think you should catch panic(string), issues 4222 and 4474 are
programming errors in the package and should not be turned into errors that lose
all the panic context.

Also, your new code loses the repanic on runtime.Error, and can lead to
inscrutable errors like "index out of range" that are totally unhelpful (lost
debugging abilities for stupid errors in user's UnmarshalJSON).

[rsc, 2012-12-17 00:50:57]

I think this is too broad a fix. Let's fix the (apparently few) panics taking
strings instead of panicking for everything.

Note that the 'case error' in your switches covers case runtime.Error too, so
this is squelching runtime.Errors that used to be panics.

I only see one panic(" in the tree that isn't unreachable:
scanner.go:610: 		panic("json: invalid use of scanner")

The test cases in issues 4474 and 4222 are real bugs in the way json uses
reflect: it shouldn't be asking reflect to do those things. We should fix those
calls, not turn reflect's panics into returned errors.

[dave_cheney.net, 2012-12-17 09:37:12]

*** Abandoned ***