Handle error responses correctly

Author: loafoe

audit/client.go

@@ -209,12 +209,7 @@ func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
 
 	response := newResponse(resp)
 
-	err = CheckResponse(resp)
-	if err != nil {
-		// even though there was an error, we still return the response
-		// in case the caller wants to inspect it further
-		return response, err
-	}
+	doErr := CheckResponse(resp)
 
 	if v != nil {
 		defer resp.Body.Close() // Only close if we plan to read it
@@ -223,16 +218,21 @@ func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
 		} else {
 			err = json.NewDecoder(resp.Body).Decode(v)
 		}
+		if err != nil {
+			return response, err
+		}
 	}
 
-	return response, err
+	return response, doErr
 }
 
 // CheckResponse checks the API response for errors, and returns them if present.
 func CheckResponse(r *http.Response) error {
 	switch r.StatusCode {
 	case 200, 201, 202, 204, 304:
 		return nil
+	case 400:
+		return ErrBadRequest
 	}
 	return ErrNonHttp20xResponse
 }

audit/create.go

@@ -7,9 +7,10 @@ import (
 	"net/http"
 
 	dstu2pb "github.com/google/fhir/go/proto/google/fhir/proto/dstu2/resources_go_proto"
+	stu3pb "github.com/google/fhir/go/proto/google/fhir/proto/stu3/resources_go_proto"
 )
 
-func (c *Client) CreateAuditEvent(event *dstu2pb.AuditEvent) (*dstu2pb.ContainedResource, *Response, error) {
+func (c *Client) CreateAuditEvent(event *dstu2pb.AuditEvent) (*stu3pb.ContainedResource, *Response, error) {
 	eventJSON, err := c.ma.MarshalResource(event)
 	if err != nil {
 		return nil, nil, err
@@ -20,22 +21,24 @@ func (c *Client) CreateAuditEvent(event *dstu2pb.AuditEvent) (*dstu2pb.Contained
 	}
 	_ = c.httpSigner.SignRequest(req)
 	var operationResponse bytes.Buffer
-	resp, err := c.Do(req, &operationResponse)
-	if (err != nil && err != io.EOF) || resp == nil {
-		if resp == nil && err != nil {
-			err = ErrEmptyResult
+	resp, doErr := c.Do(req, &operationResponse)
+	if (doErr != nil && !(doErr == io.EOF || doErr == ErrBadRequest)) || resp == nil {
+		if resp == nil && doErr != nil {
+			doErr = ErrEmptyResult
 		}
-		return nil, resp, err
+		return nil, resp, doErr
 	}
-	// Success
-	contained := &dstu2pb.ContainedResource{}
+	contained := &stu3pb.ContainedResource{}
 	if resp.StatusCode == http.StatusCreated {
 		return contained, resp, nil
 	}
 	// OperationOutcome
 	unmarshalled, err := c.um.Unmarshal(operationResponse.Bytes())
-	if err == nil {
-		contained = unmarshalled.(*dstu2pb.ContainedResource)
+	if err != nil {
+		return nil, resp, fmt.Errorf("c.um.Unmarshal: %w", err)
+	}
+	if unmarshalled != nil {
+		contained = unmarshalled.(*stu3pb.ContainedResource)
 	}
-	return contained, resp, err
+	return contained, resp, doErr
 }

audit/create_test.go

@@ -127,3 +127,56 @@ func TestCreate(t *testing.T) {
 		return
 	}
 }
+
+func TestBadRequest(t *testing.T) {
+	operationOutcome := `{
+  "issue": [
+    {
+      "severity": "error",
+      "code": "invalid",
+      "details": {
+        "coding": [
+          {
+            "system": "https://www.hl7.org/fhir/valueset-operation-outcome.html",
+            "code": "MSG_ERROR_PARSING"
+          }
+        ],
+        "text": "Not complaint with AuditEvent specification"
+      },
+      "diagnostics": "Not complaint with AuditEvent specification"
+    }
+  ],
+  "resourceType": "OperationOutcome"
+}`
+	teardown := setup(t)
+	defer teardown()
+
+	muxAudit.HandleFunc("/core/audit/AuditEvent", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.Method {
+		case "POST":
+			if !assert.Equal(t, audit.APIVersion, r.Header.Get("API-Version")) {
+				w.WriteHeader(http.StatusPreconditionFailed)
+				return
+			}
+			w.WriteHeader(http.StatusBadRequest)
+			w.Write([]byte(operationOutcome))
+		default:
+			w.WriteHeader(http.StatusMethodNotAllowed)
+		}
+	})
+	event := &dstu2pb.AuditEvent{
+		Id: &dstu2dt.Id{Value: "someID"},
+	}
+	contained, resp, err := auditClient.CreateAuditEvent(event)
+	if !assert.Equal(t, audit.ErrBadRequest, err) {
+		return
+	}
+	if !assert.NotNil(t, resp) {
+		return
+	}
+	if !assert.NotNil(t, contained) {
+		return
+	}
+
+}

audit/errors.go

@@ -10,5 +10,6 @@ var (
 	ErrEmptyResult                    = errors.New("empty result")
 	ErrCouldNoReadResourceAfterCreate = errors.New("could not read resource after create")
 	ErrNotImplementedYet              = errors.New("not implemented yet")
+	ErrBadRequest                     = errors.New("bad request")
 	ErrNonHttp20xResponse             = errors.New("non http 20x Audit response")
 )