bug/design: forwarding over `formatter` takes wrong type

[gabyx]

I suspect there is an error in generation of the Encode/Decode...Error functions. See the example below.

When I look at the generated code, I see that formatter(ctx, res) gets the casted type res (cm.CustodianError) (our error type on the method).

• This is problematic when you return a wrapped error from the endpoint: i.e. wrapping cm.CustodianError to be able to do custom stuff in a custom formatter (e.g. v is an error which Unwraps to cm.CustodianError etc).

• IMO: it should receive v, before Unwrap in As(v, &res) such that the formatter has a chance to actually read a wrapped type.

• From a design perspective I dont understand yet why body = formatter(ctx, res) returns a goahttp.Statuser because
  the returned interface goahttp.Statuser is not used as the status is forcefully set (see below):
  Maybe formatter needs to be extended to be something like:

  type ErrorFormatter interface {
      MarshalToStatuser(ctx context.Context, v any) goahttp.Statuser
      Marshal(ctx context.Context, v any) any
  }

• Also problematic is that errors.As(v, &res) is not checked and just panics (should be a debug assert).

Encode/Decode Generated:

func EncodeGetContractError(encoder func(context.Context, http.ResponseWriter) goahttp.Encoder, formatter func(ctx context.Context, err error) goahttp.Statuser) func(context.Context, http.ResponseWriter, error) error {
	encodeError := goahttp.ErrorEncoder(encoder, formatter)
	return func(ctx context.Context, w http.ResponseWriter, v error) error {
		var en goa.GoaErrorNamer
		if !errors.As(v, &en) {
			return encodeError(ctx, w, v)
		}
		switch en.GoaErrorName() {
		case "cm#not-found":
			var res *cm.CustodianError
			errors.As(v, &res)
			enc := encoder(ctx, w)
			var body any
			if formatter != nil {
				body = formatter(ctx, res)
			} else {
				body = NewGetContractCmNotFoundResponseBody(res)
			}
			w.Header().Set("goa-error", res.GoaErrorName())
			w.WriteHeader(http.StatusNotFound)
			return enc.Encode(body)

[raphael]

Hello, thank you for the feedback, let me address these points one by one:

1. Unwrapping of error: the idea is that Goa provides the user given formatter with the type of the error that's defined in the design. I can see your point about making it possible to add more context through a wrapper error and we could change the code to call formatter with v instead of res but that might be a breaking change for some.

2. You are correct that in this case the status does not matter however it is necessary in the case where an error is created dynamically and not specified in the design (allows for dynamic status codes) - this corresponds to the default case in the generated error encoder.

3. The generated code checks that the given error implements GoaErrorNamer and that the name is the name for this specific error type. Additionally the DSL validation ensures that two errors do not have the same name so the code should never panic (I guess it is theoretically possible to create a runtime type that implements GoaErrorNamer and that returns a duplicate name but in practice all error types are created from the generated code using the generated MakeXYZ methods).