From 303515ea4ab5bb1597ac9a8c0ec7457fb6b577ea Mon Sep 17 00:00:00 2001 From: Martin Gressler Date: Wed, 10 Apr 2024 15:59:03 +0200 Subject: [PATCH 1/3] Fixes opensearch-project/opensearch-go#522 Signed-off-by: Martin Gressler --- error.go | 9 +++++++-- error_test.go | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/error.go b/error.go index 5048f4bbe..8a568e8d3 100644 --- a/error.go +++ b/error.go @@ -22,6 +22,7 @@ var ( ErrUnknownOpensearchError = errors.New("opensearch error response could not be parsed as error") ErrNonJSONError = errors.New("error is not in json format") ErrUnauthorized = errors.New(http.StatusText(http.StatusUnauthorized)) + ErrTooManyRequests = errors.New(http.StatusText(http.StatusTooManyRequests)) ) // Error represents an Opensearch error with only an error field @@ -131,10 +132,14 @@ func ParseError(resp *Response) error { } if !json.Valid(body) { - if resp.StatusCode == http.StatusUnauthorized { + switch resp.StatusCode { + case http.StatusUnauthorized: return ErrUnauthorized + case http.StatusTooManyRequests: + return ErrTooManyRequests + default: + return ErrNonJSONError } - return ErrNonJSONError } var testResp struct { diff --git a/error_test.go b/error_test.go index cd9b46647..6e45c1676 100644 --- a/error_test.go +++ b/error_test.go @@ -181,6 +181,14 @@ func TestError(t *testing.T) { Resp *opensearch.Response WantedErrors []error }{ + { + Name: "Too many requests", + Resp: &opensearch.Response{ + StatusCode: http.StatusTooManyRequests, + Body: io.NopCloser(strings.NewReader(`429 Too Many Requests /testindex/_bulk`)), + }, + WantedErrors: []error{opensearch.ErrTooManyRequests}, + }, { Name: "Non JSON error", Resp: &opensearch.Response{ From bc9c8577e33b32aefbc5d69ae6bae144d5d03cb7 Mon Sep 17 00:00:00 2001 From: Martin Gressler Date: Wed, 10 Apr 2024 21:59:49 +0200 Subject: [PATCH 2/3] handle non-json error by returning body Signed-off-by: Martin Gressler --- error.go | 13 +------------ error_test.go | 43 +++++++++++++++++++------------------------ 2 files changed, 20 insertions(+), 36 deletions(-) diff --git a/error.go b/error.go index 8a568e8d3..89795f2ef 100644 --- a/error.go +++ b/error.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "io" - "net/http" ) // Error vars @@ -20,9 +19,6 @@ var ( ErrReadBody = errors.New("failed to read body") ErrJSONUnmarshalBody = errors.New("failed to json unmarshal body") ErrUnknownOpensearchError = errors.New("opensearch error response could not be parsed as error") - ErrNonJSONError = errors.New("error is not in json format") - ErrUnauthorized = errors.New(http.StatusText(http.StatusUnauthorized)) - ErrTooManyRequests = errors.New(http.StatusText(http.StatusTooManyRequests)) ) // Error represents an Opensearch error with only an error field @@ -132,14 +128,7 @@ func ParseError(resp *Response) error { } if !json.Valid(body) { - switch resp.StatusCode { - case http.StatusUnauthorized: - return ErrUnauthorized - case http.StatusTooManyRequests: - return ErrTooManyRequests - default: - return ErrNonJSONError - } + return fmt.Errorf("%s", body) } var testResp struct { diff --git a/error_test.go b/error_test.go index 6e45c1676..56dac18ae 100644 --- a/error_test.go +++ b/error_test.go @@ -181,22 +181,6 @@ func TestError(t *testing.T) { Resp *opensearch.Response WantedErrors []error }{ - { - Name: "Too many requests", - Resp: &opensearch.Response{ - StatusCode: http.StatusTooManyRequests, - Body: io.NopCloser(strings.NewReader(`429 Too Many Requests /testindex/_bulk`)), - }, - WantedErrors: []error{opensearch.ErrTooManyRequests}, - }, - { - Name: "Non JSON error", - Resp: &opensearch.Response{ - StatusCode: http.StatusMethodNotAllowed, - Body: io.NopCloser(strings.NewReader(`Test - Trigger an error`)), - }, - WantedErrors: []error{opensearch.ErrNonJSONError}, - }, { Name: "error field object", Resp: &opensearch.Response{ @@ -213,14 +197,6 @@ func TestError(t *testing.T) { }, WantedErrors: []error{opensearch.ErrUnknownOpensearchError}, }, - { - Name: "unauthorized", - Resp: &opensearch.Response{ - StatusCode: http.StatusUnauthorized, - Body: io.NopCloser(strings.NewReader(http.StatusText(http.StatusUnauthorized))), - }, - WantedErrors: []error{opensearch.ErrUnauthorized}, - }, { Name: "io read error", Resp: &opensearch.Response{ @@ -252,5 +228,24 @@ func TestError(t *testing.T) { } }) } + + t.Run("unauthorized", func(t *testing.T) { + resp := &opensearch.Response{ + StatusCode: http.StatusUnauthorized, + Body: io.NopCloser(strings.NewReader(http.StatusText(http.StatusUnauthorized))), + } + assert.True(t, resp.IsError()) + err := opensearch.ParseError(resp) + assert.Equal(t, err.Error(), http.StatusText(http.StatusUnauthorized)) + }) + t.Run("too many requests", func(t *testing.T) { + resp := &opensearch.Response{ + StatusCode: http.StatusTooManyRequests, + Body: io.NopCloser(strings.NewReader("429 Too Many Requests /testindex/_bulk")), + } + assert.True(t, resp.IsError()) + err := opensearch.ParseError(resp) + assert.Equal(t, err.Error(), "429 Too Many Requests /testindex/_bulk") + }) }) } From 7f4c2e0c81a110946c585857070fc0b2aac71b9b Mon Sep 17 00:00:00 2001 From: Martin Gressler Date: Wed, 10 Apr 2024 22:01:31 +0200 Subject: [PATCH 3/3] add to changelog Signed-off-by: Martin Gressler --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6915a29f8..7f384a4b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Adjusts tests to new opensearchapi functions and structs ([#421](https://github.com/opensearch-project/opensearch-go/pull/421)) - Changes codecov to comment code coverage to each PR ([#410](https://github.com/opensearch-project/opensearch-go/pull/410)) - Changes module version from v2 to v3 ([#444](https://github.com/opensearch-project/opensearch-go/pull/444)) +- Handle unexpected non-json errors with the response body ([#523](https://github.com/opensearch-project/opensearch-go/pull/523)) ### Deprecated