From b4ab844ca8f0e394436847146261638bf0a2e163 Mon Sep 17 00:00:00 2001 From: Jakob Date: Mon, 10 Apr 2023 23:01:48 +0200 Subject: [PATCH] opensearchapi: Fix handling of errors without error response body (#286) Signed-off-by: Jakob Hahn Signed-off-by: Rakhat Zhuman --- CHANGELOG.md | 10 +++++-- opensearchapi/opensearchapi.error.go | 2 +- opensearchapi/opensearchapi.response.go | 3 +- .../opensearchapi_response_internal_test.go | 28 ++++++++++++++++--- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a03944191..b2f20109e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,8 +22,13 @@ 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 index_template guide ([#289](https://github.com/opensearch-project/opensearch-go/pull/289)) +- Adds @Jakob3xD to co-maintainers ([#270](https://github.com/opensearch-project/opensearch-go/pull/270)) +- Adds `index_lifecycle` guide ([#287](https://github.com/opensearch-project/opensearch-go/pull/287)) +- Adds `advanced_index_actions` guide ([#288](https://github.com/opensearch-project/opensearch-go/pull/288)) +- Adds `index_template` guide ([#289](https://github.com/opensearch-project/opensearch-go/pull/289)) +- Adds `document_lifecycle` guide ([#290](https://github.com/opensearch-project/opensearch-go/pull/290)) +- Adds `search` guide ([#291](https://github.com/opensearch-project/opensearch-go/pull/291)) +- Adds `bulk` guide ([#292](https://github.com/opensearch-project/opensearch-go/pull/292)) ### Changed @@ -43,6 +48,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 cfaa155a3..5c5a30575 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 d4344ee48..0f4c561f4 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)) diff --git a/opensearchapi/opensearchapi_response_internal_test.go b/opensearchapi/opensearchapi_response_internal_test.go index 57fd6a96b..0cdc7c61f 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) }