Repository: goadesign/goa
Pull request #3398 (2023-10-19 16:13:34)

Rename the XML encoded http.ErrorResponse
Related to https://github.com/goadesign/clue/pull/284

The XML encoded http.ErrorResponse is formed as follows:

```xml
<ErrorResponse>
        <name>name</name>
        <id>id</id>
        <message>message</message>
        <temporary>false</temporary>
        <timeout>false</timeout>
        <fault>false</fault>
</ErrorResponse>
```

But I think the top level element should be like this:

```xml
<error>
        <name>name</name>
        <id>id</id>
        <message>message</message>
        <temporary>false</temporary>
        <timeout>false</timeout>
        <fault>false</fault>
</error>
```

It can be restored the previous form:


```go
goahttp.ErrorResponseXMLName = xml.Name{Local: "ErrorResponse"}
```


tchssk: DeepSource again...
raphael: Thank you, looks good. Looks like one of the issues reported by DeepSource could be addressed (https://app.deepsource.com/gh/goadesign/goa/run/68b2c44c-c727-40d1-8edf-63efacc2a6c9/go/RVV-B0012?listindex=1). The other issue was already there but maybe we can take this opportunity to fix it too (i.e. change the receivers for the Marshal methods to be pointers).

Patch:
diff --git a/http/error.go b/http/error.go
index 94d42915..79f03b7c 100644
--- a/http/error.go
+++ b/http/error.go
@@ -2,6 +2,7 @@ package http
 
 import (
 	"context"
+	"encoding/xml"
 	"net/http"
 
 	goa "goa.design/goa/v3/pkg"
@@ -35,6 +36,11 @@ type (
 	}
 )
 
+var (
+	// ErrorResponseXMLName is the XML name used for ErrorResponse.
+	ErrorResponseXMLName = xml.Name{Local: "error"}
+)
+
 // NewErrorResponse creates a HTTP response from the given error.
 func NewErrorResponse(ctx context.Context, err error) Statuser {
 	if gerr, ok := err.(*goa.ServiceError); ok {
@@ -50,6 +56,26 @@ func NewErrorResponse(ctx context.Context, err error) Statuser {
 	return NewErrorResponse(ctx, goa.Fault(err.Error()))
 }
 
+func (resp *ErrorResponse) MarshalXML(e *xml.Encoder, _ xml.StartElement) error {
+	return e.Encode(struct {
+		XMLName   xml.Name
+		Name      string `xml:"name"`
+		ID        string `xml:"id"`
+		Message   string `xml:"message"`
+		Temporary bool   `xml:"temporary"`
+		Timeout   bool   `xml:"timeout"`
+		Fault     bool   `xml:"fault"`
+	}{
+		XMLName:   ErrorResponseXMLName,
+		Name:      resp.Name,
+		ID:        resp.ID,
+		Message:   resp.Message,
+		Temporary: resp.Temporary,
+		Timeout:   resp.Timeout,
+		Fault:     resp.Fault,
+	})
+}
+
 // StatusCode implements a heuristic that computes a HTTP response status code
 // appropriate for the timeout, temporary and fault characteristics of the
 // error. This method is used by the generated server code when the error is not


Test patch:
diff --git a/http/encoding_test.go b/http/encoding_test.go
index 176c7008..caf701cd 100644
--- a/http/encoding_test.go
+++ b/http/encoding_test.go
@@ -3,10 +3,15 @@ package http
 import (
 	"bytes"
 	"context"
+	"encoding/xml"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+
+	goa "goa.design/goa/v3/pkg"
 )
 
 var (
@@ -104,6 +109,46 @@ func TestResponseEncoder(t *testing.T) {
 	}
 }
 
+func TestResponseEncoder_Encode_ErrorResponse(t *testing.T) {
+	var (
+		serviceError              = goa.NewServiceError(errors.New("foo"), "foo", false, false, false)
+		defaultXMLName            = ErrorResponseXMLName
+		backwardCompatibleXMLName = xml.Name{Local: "ErrorResponse"} // Compatible with v3.13.2 and earlier.
+	)
+
+	cases := []struct {
+		acceptType string
+		xmlName    xml.Name
+		encoded    string
+	}{
+		{"application/json", defaultXMLName, fmt.Sprintf(`{"name":"foo","id":"%s","message":"foo","temporary":false,"timeout":false,"fault":false}`, serviceError.ID)},
+		{"application/json", backwardCompatibleXMLName, fmt.Sprintf(`{"name":"foo","id":"%s","message":"foo","temporary":false,"timeout":false,"fault":false}`, serviceError.ID)},
+		{"application/xml", defaultXMLName, fmt.Sprintf(`<error><name>foo</name><id>%s</id><message>foo</message><temporary>false</temporary><timeout>false</timeout><fault>false</fault></error>`, serviceError.ID)},
+		{"application/xml", backwardCompatibleXMLName, fmt.Sprintf(`<ErrorResponse><name>foo</name><id>%s</id><message>foo</message><temporary>false</temporary><timeout>false</timeout><fault>false</fault></ErrorResponse>`, serviceError.ID)},
+	}
+
+	for _, c := range cases {
+		name := c.acceptType
+		if c.xmlName.Local != "" {
+			name += "/" + c.xmlName.Local
+		}
+		t.Run(name, func(t *testing.T) {
+			ctx := context.Background()
+			ctx = context.WithValue(ctx, AcceptTypeKey, c.acceptType)
+			w := httptest.NewRecorder()
+			ErrorResponseXMLName = c.xmlName
+			encoder := ResponseEncoder(ctx, w)
+			if err := encoder.Encode(NewErrorResponse(ctx, serviceError)); err != nil {
+				t.Error(err)
+			}
+			body := strings.TrimSpace(w.Body.String())
+			if body != c.encoded {
+				t.Errorf("got %s, expected %s", body, c.encoded)
+			}
+		})
+	}
+}
+
 func TestResponseDecoder(t *testing.T) {
 	cases := []struct {
 		contentType string