Skip to content

Commit

Permalink
opensearchapi: Fix handling of errors without error response body (#286)
Browse files Browse the repository at this point in the history
Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
Signed-off-by: Rakhat Zhuman <zhumanrakhat01@gmail.com>
  • Loading branch information
Jakob3xD authored and zethuman committed Apr 12, 2023
1 parent cffcf02 commit d3a8912
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 document_lifecycle guide ([#290](https://github.com/opensearch-project/opensearch-go/pull/290))
- 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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion opensearchapi/opensearchapi.error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
3 changes: 2 additions & 1 deletion opensearchapi/opensearchapi.response.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"io"
"io/ioutil"
"net/http"
"reflect"
"strconv"
"strings"
)
Expand Down Expand Up @@ -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))
Expand Down
28 changes: 24 additions & 4 deletions opensearchapi/opensearchapi_response_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestAPIResponse(t *testing.T) {
var (
body string
res *Response
err error
)

t.Run("String", func(t *testing.T) {
Expand Down Expand Up @@ -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(
Expand All @@ -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)
}
Expand Down

0 comments on commit d3a8912

Please sign in to comment.