fix: centralized error handling to be more central (#406)

pkg/amterror/error.go

@@ -0,0 +1,87 @@
+package amterror
+
+import (
+	"encoding/xml"
+	"fmt"
+)
+
+func (e *AMTError) Error() string {
+	return fmt.Sprintf("Error [SubCode: %s] Message: %s, Detail: %s", e.SubCode, e.Message, e.Detail)
+}
+
+func NewAMTError(subCode, message, detail string) *AMTError {
+	return &AMTError{
+		SubCode: subCode,
+		Message: message,
+		Detail:  detail,
+	}
+}
+
+func DecodeAMTErrorString(s string) error {
+	checkForErrorResponse := ErrorResponse{}
+
+	err := xml.Unmarshal([]byte(s), &checkForErrorResponse)
+	if err != nil {
+		return err
+	}
+
+	return NewAMTError(checkForErrorResponse.Body.Fault.Code.SubCode.Value, checkForErrorResponse.Body.Fault.Reason.Text, checkForErrorResponse.Body.Fault.Detail)
+}
+
+// AMT WSMAN Error Response Types.
+type (
+	AMTError struct {
+		SubCode string
+		Message string
+		Detail  string
+	}
+
+	Header struct {
+		XMLName     xml.Name `xml:"Header"`
+		To          string   `xml:"To"`
+		RelatesTo   int      `xml:"RelatesTo"`
+		Action      Action   `xml:"Action"`
+		MessageID   string   `xml:"MessageID"`
+		ResourceURI string   `xml:"ResourceURI"`
+	}
+
+	Action struct {
+		XMLName        xml.Name `xml:"Action"`
+		MustUnderstand string   `xml:"mustUnderstand,attr"`
+		Value          string   `xml:",chardata"`
+	}
+
+	ErrorResponse struct {
+		XMLName xml.Name  `xml:"Envelope"`
+		Header  Header    `xml:"Header"`
+		Body    ErrorBody `xml:"Body"`
+	}
+
+	ErrorBody struct {
+		XMLName xml.Name `xml:"Body"`
+		Fault   Fault    `xml:"Fault"`
+	}
+
+	Fault struct {
+		XMLName xml.Name `xml:"Fault"`
+		Code    Code     `xml:"Code"`
+		Reason  Reason   `xml:"Reason"`
+		Detail  string   `xml:"Detail"`
+	}
+
+	Code struct {
+		XMLName xml.Name `xml:"Code"`
+		Value   string   `xml:"Value"`
+		SubCode SubCode  `xml:"Subcode"`
+	}
+
+	SubCode struct {
+		XMLName xml.Name `xml:"Subcode"`
+		Value   string   `xml:"Value"`
+	}
+
+	Reason struct {
+		XMLName xml.Name `xml:"Reason"`
+		Text    string   `xml:"Text"`
+	}
+)

pkg/amterror/error_test.go

@@ -0,0 +1,40 @@
+/*********************************************************************
+ * Copyright (c) Intel Corporation 2024
+ * SPDX-License-Identifier: Apache-2.0
+ **********************************************************************/
+
+package amterror
+
+import (
+	"encoding/xml"
+	"testing"
+)
+
+func TestDecodeWSMANError(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected error
+	}{
+		{
+			"<?xml version=\"1.0\" encoding=\"UTF-8\"?><a:Envelope xmlns:g=\"http://schemas.dmtf.org/wbem/wsman/1/cimbinding.xsd\" xmlns:f=\"http://schemas.xmlsoap.org/ws/2004/08/eventing\" xmlns:e=\"http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd\" xmlns:d=\"http://schemas.xmlsoap.org/ws/2004/09/transfer\" xmlns:c=\"http://schemas.xmlsoap.org/ws/2004/09/enumeration\" xmlns:b=\"http://schemas.xmlsoap.org/ws/2004/08/addressing\" xmlns:a=\"http://www.w3.org/2003/05/soap-envelope\" xmlns:h=\"http://schemas.xmlsoap.org/ws/2005/02/trust\" xmlns:i=\"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"><a:Header><b:To>http://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymous</b:To><b:RelatesTo>0</b:RelatesTo><b:Action a:mustUnderstand=\"true\">http://schemas.xmlsoap.org/ws/2004/08/addressing/fault</b:Action><b:MessageID>uuid:00000000-8086-8086-8086-000000000061</b:MessageID></a:Header><a:Body><a:Fault><a:Code><a:Value>a:Sender</a:Value><a:Subcode><a:Value>b:DestinationUnreachable</a:Value></a:Subcode></a:Code><a:Reason><a:Text xml:lang=\"en-US\">No route can be determined to reach the destination role defined by the WSAddressing To.</a:Text></a:Reason><a:Detail></a:Detail></a:Fault></a:Body></a:Envelope>",
+			NewAMTError("b:DestinationUnreachable", "No route can be determined to reach the destination role defined by the WSAddressing To.", ""),
+		},
+		{
+			"bad xml",
+			xml.Unmarshal([]byte("bad xml"), &ErrorResponse{}),
+		},
+	}
+
+	for _, test := range tests {
+		result := DecodeAMTErrorString(test.input)
+		if result == nil {
+			if test.expected != nil {
+				t.Errorf("Expected %s, but got nil", test.expected)
+			}
+		} else {
+			if result.Error() != test.expected.Error() {
+				t.Errorf("Expected %s, but got %s", test.expected, result)
+			}
+		}
+	}
+}

pkg/wsman/amt/boot/settingdata.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/internal/message"
 	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/client"
-	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/common"
 )
 
 // Instantiates a new Boot Setting Data service.
@@ -148,14 +147,5 @@ func (settingData SettingData) Put(bootSettingData BootSettingDataRequest) (resp
 		return response, err
 	}
 
-	checkForErrorResponse := common.ErrorResponse{}
-
-	err = xml.Unmarshal([]byte(response.XMLOutput), &checkForErrorResponse)
-	if err != nil {
-		return response, err
-	}
-
-	err = common.DecodeAMTError(checkForErrorResponse)
-
 	return response, err
 }

pkg/wsman/cim/kvm/redirectionsap.go

@@ -12,7 +12,6 @@ import (
 	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/internal/message"
 	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/cim/methods"
 	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/client"
-	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/common"
 )
 
 // NewKVMRedirectionSAP returns a new instance of the KVMRedirectionSAP struct.
@@ -103,14 +102,5 @@ func (redirectionSAP RedirectionSAP) Pull(enumerationContext string) (response R
 		return
 	}
 
-	checkForErrorResponse := common.ErrorResponse{}
-
-	err = xml.Unmarshal([]byte(response.XMLOutput), &checkForErrorResponse)
-	if err != nil {
-		return
-	}
-
-	err = common.DecodeAMTError(checkForErrorResponse)
-
 	return
 }

pkg/wsman/client/wsman.go

@@ -22,6 +22,8 @@ import (
 	"time"
 
 	"github.com/sirupsen/logrus"
+
+	"github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/amterror"
 )
 
 const (
@@ -256,21 +258,6 @@ func (t *Target) Post(msg string) (response []byte, err error) {
 
 	defer res.Body.Close()
 
-	if res.StatusCode >= 400 {
-		b, err := io.ReadAll(res.Body)
-		if err != nil {
-			return nil, err
-		}
-
-		if t.logAMTMessages {
-			logrus.Trace(string(b))
-		}
-
-		errPostResponse := errors.New("wsman.Client post received")
-
-		return nil, fmt.Errorf("%w: %v\n%v", errPostResponse, res.Status, string(b))
-	}
-
 	response, err = io.ReadAll(res.Body)
 
 	if t.logAMTMessages {
@@ -281,6 +268,18 @@ func (t *Target) Post(msg string) (response []byte, err error) {
 		return nil, err
 	}
 
+	if res.StatusCode == 400 {
+		amterr := amterror.DecodeAMTErrorString(string(response))
+
+		return nil, amterr
+	}
+
+	if res.StatusCode >= 401 {
+		errPostResponse := errors.New("wsman.Client post received")
+
+		return nil, fmt.Errorf("%w: %v\n%v", errPostResponse, res.Status, string(response))
+	}
+
 	return response, nil
 }
 

pkg/wsman/common/constants.go

@@ -5,10 +5,6 @@
 
 package common
 
-import (
-	"fmt"
-)
-
 // TODO: Review if this file is still necessary.
 
 const ValueNotFound string = "Value not found in map"
@@ -235,23 +231,3 @@ func ConvertPackageTypeToString(value int) string {
 
 	return ValueNotFound
 }
-
-func (e *AMTError) Error() string {
-	return fmt.Sprintf("Error [SubCode: %s] Message: %s, Detail: %s", e.SubCode, e.Message, e.Detail)
-}
-
-func NewAMTError(subCode, message, detail string) *AMTError {
-	return &AMTError{
-		SubCode: subCode,
-		Message: message,
-		Detail:  detail,
-	}
-}
-
-func DecodeAMTError(er ErrorResponse) error {
-	if er.Body.Fault.Code.SubCode.Value != "" {
-		return NewAMTError(er.Body.Fault.Code.SubCode.Value, er.Body.Fault.Reason.Text, er.Body.Fault.Detail)
-	}
-
-	return nil
-}

pkg/wsman/common/types.go

@@ -30,46 +30,3 @@ type ReturnValue struct {
 	ReturnValue    int      `xml:"ReturnValue,omitempty"`
 	ReturnValueStr string   `xml:"ReturnValueStr,omitempty"`
 }
-
-// AMT WSMAN Error Response Types.
-type (
-	AMTError struct {
-		SubCode string
-		Message string
-		Detail  string
-	}
-
-	ErrorResponse struct {
-		XMLName xml.Name       `xml:"Envelope"`
-		Header  message.Header `xml:"Header"`
-		Body    ErrorBody      `xml:"Body"`
-	}
-
-	ErrorBody struct {
-		XMLName xml.Name `xml:"Body"`
-		Fault   Fault    `xml:"Fault"`
-	}
-
-	Fault struct {
-		XMLName xml.Name `xml:"Fault"`
-		Code    Code     `xml:"Code"`
-		Reason  Reason   `xml:"Reason"`
-		Detail  string   `xml:"Detail"`
-	}
-
-	Code struct {
-		XMLName xml.Name `xml:"Code"`
-		Value   string   `xml:"Value"`
-		SubCode SubCode  `xml:"Subcode"`
-	}
-
-	SubCode struct {
-		XMLName xml.Name `xml:"Subcode"`
-		Value   string   `xml:"Value"`
-	}
-
-	Reason struct {
-		XMLName xml.Name `xml:"Reason"`
-		Text    string   `xml:"Text"`
-	}
-)