Safe and Unsafe arg names can collide with method names on an error

[asanderson15]

There is bug is in the generation code, specifically the way we inline the private error type rather than naming it. Take the example of the following definition:

types:
  definitions:
    errors:
      MyError:
        code: CUSTOM_SERVER
        namespace: MyNamespace
        safe-args:
          code: integer
          message: string

The above definition would generate roughly the following:

type myError {
  Code int
  Message string
}

type MyError struct {
  errorInstanceID uuid.UUID
  myError
  cause error
  stack werror.StackTrace
}

The collision occurs on the generated SafeParams method.

// safeParams returns a set of named safe parameters detailing this particular error instance.
func (e *MyError) safeParams() map[string]interface{} {
	return map[string]interface{}{"code": e.Code, "message": e.Message, "errorInstanceId": e.errorInstanceID}
}

Because Code and Message are also methods on MyError, this actually returns functions rather than the values stored on myError.

Think the fix is just to name the internal error type on the exported error type - I don't see any reason this needs to be inlined.

[bmoylan]

Today I think you can access the struct fields directly like myErr.Message because the fields are embedded, so I think this would be a breaking change if we don't mitigate that somehow. I wonder if we should add some kind of prefix in the case of conflict with the interface methods

[asanderson15]

Interesting. In a way, that's even stranger/potentially more problematic, since the internal fields would also be shaded by the Message and Code methods whenever a collision occurs.

A simple non-breaking workaround for the safe params issue would be:

return map[string]interface{}{"code": e.myError.Code, "message": e.myError.Message, "errorInstanceId": e.errorInstanceID}

I'll probably contribute ^ either way when I get a chance. But at some point we probably need to revisit and potentially do a break to address all the potential collisions. There is at least one other issue tracked for a name collision on generated errors (e.g., #78) and I bet I could think of a couple more if I took a harder look.