Provide more granular error messages to template parsing

[xdu31]

The error returned from goformation.Open. Now when I open a CloudFormation template, it only gives me an error message as:

json: cannot unmarshal string into Go struct field Template.Resources of type []string

There's no additional information on where the error is coming from, since the Template.Resources can be quite long.

[sspaink]

I looked deeper into this, and I think the problem has to do with the encoding/json package and custom UnmarshalJson not sharing error contexts. So I walked through the code in debug and it seems json.Unmarshal returns the custom error type UnmarshalTypeError which has a Struct and Field, when the actual error occurs in the custom UnmarshalJson the context is more granular but right before the error returns the method addErrorContext gets called and overwrites Struct to be Template and Field to become Resources. It doesn't check if Struct and Field had already been set, so information gets lost and leads to the unhelpful error message.

I wonder if the right path forward would be to have the custom UnmarshalJson for the resources return a new error type that has more details and then the method addErrorContext won't overwrite anything because it only works on the UnmarshalTypeError. Otherwise maybe submitting a PR to the encoding/json package to check if Struct and Field are nil before overwriting them could help, but not entirely sure if this is intended behavior. Thoughts on this?

[rubenfonseca]

@sspaink thank you so much for the deep investigation. Would you fancy to write a PR with your proposed change? It would be a great addition.

[sspaink]

I'd be happy to create a PR, I suppose trying to update the addErrorContext method to check before overwriting the context in the encoding/json package might make sense as a first attempt? and see what the Go team thinks about it. If that doesn't pan out I can try creating a PR to update the goformation package to support a different error type.

[rubenfonseca]

@sspaink that would be amazing if the Go team would accept it. However, remember that might take years before customers adopt the next version of Go, which leaves our problem unsolved for a long time. So we have to keep in mind implementing the second option anyway.

[xdu31]

Thanks for looking into this @rubenfonseca @sspaink