Repository: opensearch-project/opensearch-go
Pull request #286 (2023-04-07 12:19:36)

opensearchapi: Fix handling of errors without error response body
### Description
- Check if parsed error is empty
- Improve error string output
- Adds test for errors without an error response body

### Issues Resolved
Closes https://github.com/opensearch-project/opensearch-go/issues/281

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5dcc0ad..2ef5ba9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,7 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Adds InfoResp type ([#253](https://github.com/opensearch-project/opensearch-go/pull/253))
 - Adds markdown linter ([#261](https://github.com/opensearch-project/opensearch-go/pull/261))
 - Adds testcases to check upsert functionality ([#207](https://github.com/opensearch-project/opensearch-go/issues/207))
-- Added @Jakob3xD to co-maintainers ([#270](https://github.com/opensearch-project/opensearch-go/pull/270))
+- Adds @Jakob3xD to co-maintainers ([#270](https://github.com/opensearch-project/opensearch-go/pull/270))
 
 ### Changed
 
@@ -42,6 +42,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Renames the sequence number struct tag to `if_seq_no` to fix optimistic concurrency control ([#166](https://github.com/opensearch-project/opensearch-go/pull/166))
 - Fixes `RetryOnConflict` on bulk indexer ([#215](https://github.com/opensearch-project/opensearch-go/pull/215))
 - Corrects curl logging to emit the correct URL destination ([#101](https://github.com/opensearch-project/opensearch-go/pull/101))
+- Corrects handling of errors without an error response body ([#286](https://github.com/opensearch-project/opensearch-go/pull/286))
 
 ### Security
 
diff --git a/opensearchapi/opensearchapi.error.go b/opensearchapi/opensearchapi.error.go
index cfaa155..5c5a305 100644
--- a/opensearchapi/opensearchapi.error.go
+++ b/opensearchapi/opensearchapi.error.go
@@ -49,5 +49,5 @@ type RootCause struct {
 
 // Error returns a string.
 func (e *Error) Error() string {
-	return fmt.Sprintf("error: %s, status: %d", e.Err, e.Status)
+	return fmt.Sprintf("status: %d, type: %s, reason: %s, root_cause: %s", e.Status, e.Err.Type, e.Err.Reason, e.Err.RootCause)
 }
diff --git a/opensearchapi/opensearchapi.response.go b/opensearchapi/opensearchapi.response.go
index d4344ee..0f4c561 100644
--- a/opensearchapi/opensearchapi.response.go
+++ b/opensearchapi/opensearchapi.response.go
@@ -33,6 +33,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"reflect"
 	"strconv"
 	"strings"
 )
@@ -120,7 +121,7 @@ func (r *Response) Err() error {
 		}
 		var e *Error
 		err = json.Unmarshal(body, &e)
-		if err == nil {
+		if err == nil && !reflect.ValueOf(e.Err).IsZero() {
 			return e
 		}
 		return fmt.Errorf("status: %d, error: %s", r.StatusCode, string(body))


Test patch:
diff --git a/opensearchapi/opensearchapi_response_internal_test.go b/opensearchapi/opensearchapi_response_internal_test.go
index 57fd6a9..0cdc7c6 100644
--- a/opensearchapi/opensearchapi_response_internal_test.go
+++ b/opensearchapi/opensearchapi_response_internal_test.go
@@ -46,6 +46,7 @@ func TestAPIResponse(t *testing.T) {
 	var (
 		body string
 		res  *Response
+		err  error
 	)
 
 	t.Run("String", func(t *testing.T) {
@@ -108,16 +109,36 @@ func TestAPIResponse(t *testing.T) {
 	t.Run("Error", func(t *testing.T) {
 		res = &Response{StatusCode: 201}
 
-		if err := res.Err(); err != nil {
+		if err = res.Err(); err != nil {
 			t.Errorf("Unexpected error for response: %s", res.Status())
 		}
 
 		res = &Response{StatusCode: 403}
 
-		if err := res.Err(); err == nil {
+		if err = res.Err(); err == nil {
 			t.Errorf("Expected error for response: %s", res.Status())
 		}
 
+		res = &Response{
+			StatusCode: 404,
+			Body: io.NopCloser(
+				strings.NewReader(`
+				{
+					"_index":"index",
+					"_id":"2",
+					"matched":false
+				}`),
+			),
+		}
+		err = res.Err()
+		if err == nil {
+			t.Errorf("Expected error for response: %s", res.Status())
+		}
+		var errTest *Error
+		if errors.As(err, &errTest) {
+			t.Errorf("Expected error NOT to be of type opensearchapi.Error: %T", err)
+		}
+
 		res = &Response{
 			StatusCode: 400,
 			Body: io.NopCloser(
@@ -139,11 +160,10 @@ func TestAPIResponse(t *testing.T) {
 				}`)),
 		}
 
-		err := res.Err()
+		err = res.Err()
 		if err == nil {
 			t.Errorf("Expected error for response: %s", res.Status())
 		}
-		var errTest *Error
 		if !errors.As(err, &errTest) {
 			t.Errorf("Expected error to be of type opensearchapi.Error: %T", err)
 		}