From 97e14d349b5a3b9b60cb42e2e407322d4891dfdd Mon Sep 17 00:00:00 2001 From: karel rehor Date: Fri, 2 Aug 2024 15:11:53 +0200 Subject: [PATCH 01/13] feat: adds HTTP response headers to Error type --- api/http/error.go | 3 +++ api/http/service.go | 22 +++++++++++++++++++++- client_e2e_test.go | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/api/http/error.go b/api/http/error.go index 2cbcae45..04de165d 100644 --- a/api/http/error.go +++ b/api/http/error.go @@ -6,6 +6,7 @@ package http import ( "fmt" + "net/http" "strconv" ) @@ -16,6 +17,7 @@ type Error struct { Message string Err error RetryAfter uint + Header http.Header } // Error fulfils error interface @@ -45,5 +47,6 @@ func NewError(err error) *Error { Message: "", Err: err, RetryAfter: 0, + Header: http.Header{}, } } diff --git a/api/http/service.go b/api/http/service.go index 7e2bc466..3cb6f31b 100644 --- a/api/http/service.go +++ b/api/http/service.go @@ -16,6 +16,7 @@ package http import ( "context" "encoding/json" + "fmt" "io" "mime" "net/http" @@ -113,7 +114,25 @@ func (s *service) DoHTTPRequest(req *http.Request, requestCallback RequestCallba } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return s.parseHTTPError(resp) + logHeaders := "" + httpErr := s.parseHTTPError(resp) + for _, candidate := range []string{ + "date", + "trace-id", + "trace-sampled", + "X-Influxdb-Build", + "X-Influxdb-Request-ID", + "X-Influxdb-Version", + } { + if httpErr.Header.Get(candidate) != "" { + logHeaders += fmt.Sprintf("%s: %s\n", candidate, httpErr.Header.Get(candidate)) + } + } + log.Errorf("http status code: %d, %s\nSelect Response Headers:\n%s", + resp.StatusCode, + httpErr.Error(), + logHeaders) + return httpErr } if responseCallback != nil { err := responseCallback(resp) @@ -151,6 +170,7 @@ func (s *service) parseHTTPError(r *http.Response) *Error { perror := NewError(nil) perror.StatusCode = r.StatusCode + perror.Header = r.Header if v := r.Header.Get("Retry-After"); v != "" { r, err := strconv.ParseUint(v, 10, 32) diff --git a/client_e2e_test.go b/client_e2e_test.go index a604109a..9245f031 100644 --- a/client_e2e_test.go +++ b/client_e2e_test.go @@ -18,6 +18,7 @@ import ( "time" influxdb2 "github.com/influxdata/influxdb-client-go/v2" + "github.com/influxdata/influxdb-client-go/v2/api/http" "github.com/influxdata/influxdb-client-go/v2/domain" "github.com/influxdata/influxdb-client-go/v2/internal/test" "github.com/influxdata/influxdb-client-go/v2/log" @@ -368,3 +369,16 @@ func TestWriteCustomBatch(t *testing.T) { } assert.Equal(t, 10, l) } + +func TestHttpHeadersInError(t *testing.T) { + client := influxdb2.NewClientWithOptions(serverURL, authToken, influxdb2.DefaultOptions().SetLogLevel(0)) + err := client.WriteAPIBlocking("my-org", "my-bucket").WriteRecord(context.Background(), "asdf") + assert.Error(t, err) + assert.Len(t, err.(*http.Error).Header, 6) + assert.NotEqual(t, err.(*http.Error).Header.Get("Date"), "") + assert.NotEqual(t, err.(*http.Error).Header.Get("Content-Length"), "") + assert.NotEqual(t, err.(*http.Error).Header.Get("Content-Type"), "") + assert.NotEqual(t, err.(*http.Error).Header.Get("X-Platform-Error-Code"), "") + assert.Contains(t, err.(*http.Error).Header.Get("X-Influxdb-Version"), "v") + assert.Equal(t, err.(*http.Error).Header.Get("X-Influxdb-Build"), "OSS") +} From e087f4d76e1e51f8c038f3a6279a08c0b64dd013 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Tue, 6 Aug 2024 17:12:57 +0200 Subject: [PATCH 02/13] feat: add http/Error to error Channel making Error.Header accessible --- api/http/error.go | 19 ++++++++++++++ api/http/service.go | 21 +-------------- api/write_test.go | 47 ++++++++++++++++++++++++++++++++++ internal/write/service.go | 26 +++++++++++++++++-- internal/write/service_test.go | 20 +++++++++++++++ 5 files changed, 111 insertions(+), 22 deletions(-) diff --git a/api/http/error.go b/api/http/error.go index 04de165d..e7ab0cd6 100644 --- a/api/http/error.go +++ b/api/http/error.go @@ -27,6 +27,8 @@ func (e *Error) Error() string { return e.Err.Error() case e.Code != "" && e.Message != "": return fmt.Sprintf("%s: %s", e.Code, e.Message) + case e.Message != "": + return e.Message default: return "Unexpected status code " + strconv.Itoa(e.StatusCode) } @@ -39,6 +41,23 @@ func (e *Error) Unwrap() error { return nil } +// HeaderToString generates a string value from the Header property. Useful in logging. +func (e *Error) HeaderToString(selected []string) string { + headerString := "" + if len(selected) == 0 { + for key := range e.Header { + headerString += fmt.Sprintf("%s: %s\r\n", key, e.Header[key]) + } + } else { + for _, candidate := range selected { + if e.Header.Get(candidate) != "" { + headerString += fmt.Sprintf("%s: %s\n", candidate, e.Header.Get(candidate)) + } + } + } + return headerString +} + // NewError returns newly created Error initialised with nested error and default values func NewError(err error) *Error { return &Error{ diff --git a/api/http/service.go b/api/http/service.go index 3cb6f31b..ef1b8d13 100644 --- a/api/http/service.go +++ b/api/http/service.go @@ -16,7 +16,6 @@ package http import ( "context" "encoding/json" - "fmt" "io" "mime" "net/http" @@ -114,25 +113,7 @@ func (s *service) DoHTTPRequest(req *http.Request, requestCallback RequestCallba } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - logHeaders := "" - httpErr := s.parseHTTPError(resp) - for _, candidate := range []string{ - "date", - "trace-id", - "trace-sampled", - "X-Influxdb-Build", - "X-Influxdb-Request-ID", - "X-Influxdb-Version", - } { - if httpErr.Header.Get(candidate) != "" { - logHeaders += fmt.Sprintf("%s: %s\n", candidate, httpErr.Header.Get(candidate)) - } - } - log.Errorf("http status code: %d, %s\nSelect Response Headers:\n%s", - resp.StatusCode, - httpErr.Error(), - logHeaders) - return httpErr + return s.parseHTTPError(resp) } if responseCallback != nil { err := responseCallback(resp) diff --git a/api/write_test.go b/api/write_test.go index 7ddbab19..4fda86bd 100644 --- a/api/write_test.go +++ b/api/write_test.go @@ -8,7 +8,10 @@ import ( "fmt" "io" "math" + ihttp "net/http" + "net/http/httptest" "runtime" + "strconv" "strings" "sync" "testing" @@ -265,3 +268,47 @@ func TestFlushWithRetries(t *testing.T) { // two remained assert.Equal(t, 2, len(service.Lines())) } + +func TestWriteApiErrorHeaders(t *testing.T) { + calls := 0 + var mu sync.Mutex + server := httptest.NewServer(ihttp.HandlerFunc(func(w ihttp.ResponseWriter, r *ihttp.Request) { + mu.Lock() + defer mu.Unlock() + calls++ + w.Header().Set("X-Test-Val1", "Not All Correct") + w.Header().Set("X-Test-Val2", "Atlas LV-3B") + w.Header().Set("X-Call-Count", strconv.Itoa(calls)) + w.WriteHeader(ihttp.StatusBadRequest) + _, _ = w.Write([]byte(`{ "code": "bad request", "message": "test header" }`)) + })) + defer server.Close() + svc := http.NewService(server.URL, "my-token", http.DefaultOptions()) + writeAPI := NewWriteAPI("my-org", "my-bucket", svc, write.DefaultOptions().SetBatchSize(5)) + defer writeAPI.Close() + errCh := writeAPI.Errors() + var wg sync.WaitGroup + var recErr error + wg.Add(1) + go func() { + for i := 0; i < 3; i++ { + recErr = <-errCh + assert.NotNil(t, recErr, "errCh should not run out of values") + assert.Len(t, recErr.(*http.Error).Header, 6) + assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Date")) + assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Content-Length")) + assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Content-Type")) + assert.Equal(t, strconv.Itoa(i+1), recErr.(*http.Error).Header.Get("X-Call-Count")) + assert.Equal(t, "Not All Correct", recErr.(*http.Error).Header.Get("X-Test-Val1")) + assert.Equal(t, "Atlas LV-3B", recErr.(*http.Error).Header.Get("X-Test-Val2")) + } + wg.Done() + }() + points := test.GenPoints(15) + for i := 0; i < 15; i++ { + writeAPI.WritePoint(points[i]) + } + writeAPI.waitForFlushing() + wg.Wait() + assert.Equal(t, calls, 3) +} diff --git a/internal/write/service.go b/internal/write/service.go index e47acf57..f2c8ac53 100644 --- a/internal/write/service.go +++ b/internal/write/service.go @@ -164,6 +164,7 @@ func (w *Service) HandleWrite(ctx context.Context, batch *Batch) error { if batchToWrite != nil { perror := w.WriteBatch(ctx, batchToWrite) if perror != nil { + // fmt.Printf("DEBUG perror type %s\n", reflect.TypeOf(perror)) if isIgnorableError(perror) { log.Warnf("Write error: %s", perror.Error()) } else { @@ -196,9 +197,30 @@ func (w *Service) HandleWrite(ctx context.Context, batch *Batch) error { w.retryAttempts++ log.Debugf("Write proc: next wait for write is %dms\n", w.retryDelay) } else { - log.Errorf("Write error: %s\n", perror.Error()) + logMessage := fmt.Sprintf("Write error: %s", perror.Error()) + logHeaders := perror.HeaderToString([]string{ + "date", + "trace-id", + "trace-sampled", + "X-Influxdb-Build", + "X-Influxdb-Request-ID", + "X-Influxdb-Version", + }) + if len(logHeaders) > 0 { + logMessage += fmt.Sprintf("\nSelect Response Headers:\n%s", logHeaders) + } + log.Error(logMessage) + } + return &http2.Error{ + StatusCode: int(perror.StatusCode), + Code: perror.Code, + Message: fmt.Errorf( + "write failed (attempts %d): %w", batchToWrite.RetryAttempts, perror, + ).Error(), + Err: perror.Err, + RetryAfter: perror.RetryAfter, + Header: perror.Header, } - return fmt.Errorf("write failed (attempts %d): %w", batchToWrite.RetryAttempts, perror) } } diff --git a/internal/write/service_test.go b/internal/write/service_test.go index 6214a9be..e9af3931 100644 --- a/internal/write/service_test.go +++ b/internal/write/service_test.go @@ -337,10 +337,13 @@ func TestMaxRetryTime(t *testing.T) { b := NewBatch("2\n", exp) // First batch will be checked against maxRetryTime and it will expire. New batch will fail and it will added to retry queue err = srv.HandleWrite(ctx, b) + //fmt.Printf("DEBUG err %v\n", err) + //fmt.Printf("DEBUG err %v\n", err) require.NotNil(t, err) // 1st Batch expires and writing 2nd trows error assert.Equal(t, "write failed (attempts 1): Unexpected status code 429", err.Error()) assert.Equal(t, 1, srv.retryQueue.list.Len()) + // fmt.Printf("DEBUG Header len: %d\n", len(err.(*http.Error).Header)) //wait until remaining accumulated retryDelay has passed, because there hasn't been a successful write yet <-time.After(time.Until(srv.lastWriteAttempt.Add(time.Millisecond * time.Duration(srv.retryDelay)))) @@ -702,3 +705,20 @@ func TestIgnoreErrors(t *testing.T) { err = srv.HandleWrite(ctx, b) assert.Error(t, err) } + +func TestHttpErrorHeaders(t *testing.T) { + server := httptest.NewServer(ihttp.HandlerFunc(func(w ihttp.ResponseWriter, r *ihttp.Request) { + w.Header().Set("X-Test-Val1", "Not All Correct") + w.Header().Set("X-Test-Val2", "Atlas LV-3B") + w.WriteHeader(ihttp.StatusBadRequest) + _, _ = w.Write([]byte(`{ "code": "bad request", "message": "test header" }`)) + })) + defer server.Close() + svc := NewService("my-org", "my-bucket", http.NewService(server.URL, "", http.DefaultOptions()), + write.DefaultOptions()) + err := svc.HandleWrite(context.Background(), NewBatch("1", 20)) + assert.Error(t, err) + assert.Equal(t, "400 Bad Request: write failed (attempts 0): 400 Bad Request: { \"code\": \"bad request\", \"message\": \"test header\" }", err.Error()) + assert.Equal(t, "Not All Correct", err.(*http.Error).Header.Get("X-Test-Val1")) + assert.Equal(t, "Atlas LV-3B", err.(*http.Error).Header.Get("X-Test-Val2")) +} From 820377650c971f5159e00374eb9acd4e5bd49920 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Wed, 7 Aug 2024 10:22:00 +0200 Subject: [PATCH 03/13] docs: update ExampleWriteApi_errors to show use of Header field --- api/examples_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/examples_test.go b/api/examples_test.go index b9f50d55..01853c6d 100644 --- a/api/examples_test.go +++ b/api/examples_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/influxdata/influxdb-client-go/v2/api" + apiHttp "github.com/influxdata/influxdb-client-go/v2/api/http" "github.com/influxdata/influxdb-client-go/v2/api/write" "github.com/influxdata/influxdb-client-go/v2/domain" influxdb2 "github.com/influxdata/influxdb-client-go/v2/internal/examples" @@ -123,6 +124,7 @@ func ExampleWriteAPI_errors() { go func() { for err := range errorsCh { fmt.Printf("write error: %s\n", err.Error()) + fmt.Printf("trace-id: %s\n", err.(*apiHttp.Error).Header.Get("Trace-ID")) } }() // write some points From a83a3866002a15ef958dcb96dff08bc8c33d93fb Mon Sep 17 00:00:00 2001 From: karel rehor Date: Wed, 7 Aug 2024 10:48:33 +0200 Subject: [PATCH 04/13] docs: updates CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23d692c2..9d2713fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.14 [unreleased] + +### Features + +- [#404](https://github.com/influxdata/influxdb-client-go/pull/404) Expose HTTP response headers in the Error type to aid analysis and debugging of error results. Add selected response headers to the error log. + ## 2.13.0 [2023-12-05] ### Features From 758784a3225a0d47c9ef1de16fd161c5eea73746 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Wed, 7 Aug 2024 13:52:09 +0200 Subject: [PATCH 05/13] chore: improve Error.HeaderToString and cover with test. --- api/http/error.go | 8 ++++++-- api/write_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/api/http/error.go b/api/http/error.go index e7ab0cd6..eb46827b 100644 --- a/api/http/error.go +++ b/api/http/error.go @@ -46,12 +46,16 @@ func (e *Error) HeaderToString(selected []string) string { headerString := "" if len(selected) == 0 { for key := range e.Header { - headerString += fmt.Sprintf("%s: %s\r\n", key, e.Header[key]) + headerString += fmt.Sprintf("%s: %s\r\n", + http.CanonicalHeaderKey(key), + e.Header.Get(key)) } } else { for _, candidate := range selected { if e.Header.Get(candidate) != "" { - headerString += fmt.Sprintf("%s: %s\n", candidate, e.Header.Get(candidate)) + headerString += fmt.Sprintf("%s: %s\n", + http.CanonicalHeaderKey(candidate), + e.Header.Get(candidate)) } } } diff --git a/api/write_test.go b/api/write_test.go index 4fda86bd..a3c65407 100644 --- a/api/write_test.go +++ b/api/write_test.go @@ -312,3 +312,44 @@ func TestWriteApiErrorHeaders(t *testing.T) { wg.Wait() assert.Equal(t, calls, 3) } + +func TestWriteErrorHeaderToString(t *testing.T) { + header := ihttp.Header{ + "Date": []string{"2024-08-07T12:00:00.009"}, + "Content-Length": []string{"12"}, + "Content-Type": []string{"application/json", "encoding UTF-8"}, + "X-Test-Value1": []string{"SaturnV"}, + "X-Test-Value2": []string{"Apollo11"}, + "Retry-After": []string{"2044"}, + "Trace-Id": []string{"123456789ABCDEF0"}, + } + + err := http.Error{ + StatusCode: ihttp.StatusBadRequest, + Code: "bad request", + Message: "this is just a test", + Err: nil, + RetryAfter: 2044, + Header: header, + } + + fullString := err.HeaderToString([]string{}) + + // write order is not guaranteed + assert.Contains(t, fullString, "Date: 2024-08-07T12:00:00.009") + assert.Contains(t, fullString, "Content-Length: 12") + assert.Contains(t, fullString, "Content-Type: application/json") + assert.Contains(t, fullString, "X-Test-Value1: SaturnV") + assert.Contains(t, fullString, "X-Test-Value2: Apollo11") + assert.Contains(t, fullString, "Retry-After: 2044") + assert.Contains(t, fullString, "Trace-Id: 123456789ABCDEF0") + + filterString := err.HeaderToString([]string{"date", "trace-id", "x-test-value1", "x-test-value2"}) + + // write order will follow filter arguments + assert.Equal(t, filterString, + "Date: 2024-08-07T12:00:00.009\nTrace-Id: 123456789ABCDEF0\nX-Test-Value1: SaturnV\nX-Test-Value2: Apollo11\n", + ) + assert.NotContains(t, filterString, "Content-Type: application/json") + assert.NotContains(t, filterString, "Retry-After: 2044") +} From 0da64b32d763d990ad0c4432484763a8ccdff8e0 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Wed, 7 Aug 2024 14:24:47 +0200 Subject: [PATCH 06/13] chore: better leverage http.CanonicalHeaderKey in Error.HeaderToString() --- api/http/error.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/api/http/error.go b/api/http/error.go index eb46827b..53a91a0b 100644 --- a/api/http/error.go +++ b/api/http/error.go @@ -46,16 +46,14 @@ func (e *Error) HeaderToString(selected []string) string { headerString := "" if len(selected) == 0 { for key := range e.Header { - headerString += fmt.Sprintf("%s: %s\r\n", - http.CanonicalHeaderKey(key), - e.Header.Get(key)) + k := http.CanonicalHeaderKey(key) + headerString += fmt.Sprintf("%s: %s\r\n", k, e.Header.Get(k)) } } else { for _, candidate := range selected { - if e.Header.Get(candidate) != "" { - headerString += fmt.Sprintf("%s: %s\n", - http.CanonicalHeaderKey(candidate), - e.Header.Get(candidate)) + c := http.CanonicalHeaderKey(candidate) + if e.Header.Get(c) != "" { + headerString += fmt.Sprintf("%s: %s\n", c, e.Header.Get(c)) } } } From 4e3ff5f599856331f5f7b2c6d1824befe1d07df5 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Thu, 8 Aug 2024 10:11:39 +0200 Subject: [PATCH 07/13] test: refactor to error_test.go and remove commented lines --- api/http/error_test.go | 53 ++++++++++++++++++++++++++++++++++ api/write_test.go | 41 -------------------------- internal/write/service.go | 3 +- internal/write/service_test.go | 3 -- 4 files changed, 54 insertions(+), 46 deletions(-) create mode 100644 api/http/error_test.go diff --git a/api/http/error_test.go b/api/http/error_test.go new file mode 100644 index 00000000..b21b2de4 --- /dev/null +++ b/api/http/error_test.go @@ -0,0 +1,53 @@ +// Copyright 2020-2021 InfluxData, Inc. All rights reserved. +// Use of this source code is governed by MIT +// license that can be found in the LICENSE file. + +package http + +import ( + "github.com/stretchr/testify/assert" + ihttp "net/http" + + "testing" +) + +func TestWriteErrorHeaderToString(t *testing.T) { + header := ihttp.Header{ + "Date": []string{"2024-08-07T12:00:00.009"}, + "Content-Length": []string{"12"}, + "Content-Type": []string{"application/json", "encoding UTF-8"}, + "X-Test-Value1": []string{"SaturnV"}, + "X-Test-Value2": []string{"Apollo11"}, + "Retry-After": []string{"2044"}, + "Trace-Id": []string{"123456789ABCDEF0"}, + } + + err := Error{ + StatusCode: ihttp.StatusBadRequest, + Code: "bad request", + Message: "this is just a test", + Err: nil, + RetryAfter: 2044, + Header: header, + } + + fullString := err.HeaderToString([]string{}) + + // write order is not guaranteed + assert.Contains(t, fullString, "Date: 2024-08-07T12:00:00.009") + assert.Contains(t, fullString, "Content-Length: 12") + assert.Contains(t, fullString, "Content-Type: application/json") + assert.Contains(t, fullString, "X-Test-Value1: SaturnV") + assert.Contains(t, fullString, "X-Test-Value2: Apollo11") + assert.Contains(t, fullString, "Retry-After: 2044") + assert.Contains(t, fullString, "Trace-Id: 123456789ABCDEF0") + + filterString := err.HeaderToString([]string{"date", "trace-id", "x-test-value1", "x-test-value2"}) + + // write order will follow filter arguments + assert.Equal(t, filterString, + "Date: 2024-08-07T12:00:00.009\nTrace-Id: 123456789ABCDEF0\nX-Test-Value1: SaturnV\nX-Test-Value2: Apollo11\n", + ) + assert.NotContains(t, filterString, "Content-Type: application/json") + assert.NotContains(t, filterString, "Retry-After: 2044") +} diff --git a/api/write_test.go b/api/write_test.go index a3c65407..4fda86bd 100644 --- a/api/write_test.go +++ b/api/write_test.go @@ -312,44 +312,3 @@ func TestWriteApiErrorHeaders(t *testing.T) { wg.Wait() assert.Equal(t, calls, 3) } - -func TestWriteErrorHeaderToString(t *testing.T) { - header := ihttp.Header{ - "Date": []string{"2024-08-07T12:00:00.009"}, - "Content-Length": []string{"12"}, - "Content-Type": []string{"application/json", "encoding UTF-8"}, - "X-Test-Value1": []string{"SaturnV"}, - "X-Test-Value2": []string{"Apollo11"}, - "Retry-After": []string{"2044"}, - "Trace-Id": []string{"123456789ABCDEF0"}, - } - - err := http.Error{ - StatusCode: ihttp.StatusBadRequest, - Code: "bad request", - Message: "this is just a test", - Err: nil, - RetryAfter: 2044, - Header: header, - } - - fullString := err.HeaderToString([]string{}) - - // write order is not guaranteed - assert.Contains(t, fullString, "Date: 2024-08-07T12:00:00.009") - assert.Contains(t, fullString, "Content-Length: 12") - assert.Contains(t, fullString, "Content-Type: application/json") - assert.Contains(t, fullString, "X-Test-Value1: SaturnV") - assert.Contains(t, fullString, "X-Test-Value2: Apollo11") - assert.Contains(t, fullString, "Retry-After: 2044") - assert.Contains(t, fullString, "Trace-Id: 123456789ABCDEF0") - - filterString := err.HeaderToString([]string{"date", "trace-id", "x-test-value1", "x-test-value2"}) - - // write order will follow filter arguments - assert.Equal(t, filterString, - "Date: 2024-08-07T12:00:00.009\nTrace-Id: 123456789ABCDEF0\nX-Test-Value1: SaturnV\nX-Test-Value2: Apollo11\n", - ) - assert.NotContains(t, filterString, "Content-Type: application/json") - assert.NotContains(t, filterString, "Retry-After: 2044") -} diff --git a/internal/write/service.go b/internal/write/service.go index f2c8ac53..e39acffb 100644 --- a/internal/write/service.go +++ b/internal/write/service.go @@ -164,7 +164,6 @@ func (w *Service) HandleWrite(ctx context.Context, batch *Batch) error { if batchToWrite != nil { perror := w.WriteBatch(ctx, batchToWrite) if perror != nil { - // fmt.Printf("DEBUG perror type %s\n", reflect.TypeOf(perror)) if isIgnorableError(perror) { log.Warnf("Write error: %s", perror.Error()) } else { @@ -207,7 +206,7 @@ func (w *Service) HandleWrite(ctx context.Context, batch *Batch) error { "X-Influxdb-Version", }) if len(logHeaders) > 0 { - logMessage += fmt.Sprintf("\nSelect Response Headers:\n%s", logHeaders) + logMessage += fmt.Sprintf("\nSelected Response Headers:\n%s", logHeaders) } log.Error(logMessage) } diff --git a/internal/write/service_test.go b/internal/write/service_test.go index e9af3931..ed0b077e 100644 --- a/internal/write/service_test.go +++ b/internal/write/service_test.go @@ -337,13 +337,10 @@ func TestMaxRetryTime(t *testing.T) { b := NewBatch("2\n", exp) // First batch will be checked against maxRetryTime and it will expire. New batch will fail and it will added to retry queue err = srv.HandleWrite(ctx, b) - //fmt.Printf("DEBUG err %v\n", err) - //fmt.Printf("DEBUG err %v\n", err) require.NotNil(t, err) // 1st Batch expires and writing 2nd trows error assert.Equal(t, "write failed (attempts 1): Unexpected status code 429", err.Error()) assert.Equal(t, 1, srv.retryQueue.list.Len()) - // fmt.Printf("DEBUG Header len: %d\n", len(err.(*http.Error).Header)) //wait until remaining accumulated retryDelay has passed, because there hasn't been a successful write yet <-time.After(time.Until(srv.lastWriteAttempt.Add(time.Millisecond * time.Duration(srv.retryDelay)))) From af74d7a1624a852ccf45609d96e4e4378f4b2e6c Mon Sep 17 00:00:00 2001 From: karel rehor Date: Thu, 8 Aug 2024 14:59:07 +0200 Subject: [PATCH 08/13] feat: create write/Error wrapper to better handle http/Error with Headers. --- api/http/error_test.go | 2 +- api/write/error.go | 64 +++++++++++++++++++++++++++++++++ api/write/error_test.go | 66 ++++++++++++++++++++++++++++++++++ api/write_test.go | 16 +++++---- internal/write/service.go | 11 +----- internal/write/service_test.go | 8 ++--- 6 files changed, 145 insertions(+), 22 deletions(-) create mode 100644 api/write/error.go create mode 100644 api/write/error_test.go diff --git a/api/http/error_test.go b/api/http/error_test.go index b21b2de4..ec5dc180 100644 --- a/api/http/error_test.go +++ b/api/http/error_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 InfluxData, Inc. All rights reserved. +// Copyright 2020-2024 InfluxData, Inc. All rights reserved. // Use of this source code is governed by MIT // license that can be found in the LICENSE file. diff --git a/api/write/error.go b/api/write/error.go new file mode 100644 index 00000000..f8c11865 --- /dev/null +++ b/api/write/error.go @@ -0,0 +1,64 @@ +// Copyright 2020-2024 InfluxData, Inc. All rights reserved. +// Use of this source code is governed by MIT +// license that can be found in the LICENSE file. + +package write + +import ( + "errors" + "fmt" + "github.com/influxdata/influxdb-client-go/v2/api/http" + iHttp "net/http" + "net/textproto" +) + +// Error wraps an error that may have occurred during a write call. Most often this will be an http.Error. +type Error struct { + origin error + message string +} + +// NewError returns a new created Error instance wrapping the original error. +func NewError(origin error, message string) *Error { + return &Error{ + origin: origin, + message: message, + } +} + +// Error fulfills the error interface +func (e *Error) Error() string { + return fmt.Sprintf("%s:\n %s", e.message, e.origin) +} + +func (e *Error) Unwrap() error { + return e.origin +} + +// HTTPHeader returns the Header of a wrapped http.Error. If the original error is not http.Error returns standard error. +func (e *Error) HTTPHeader() (iHttp.Header, error) { + var err *http.Error + ok := errors.As(e.origin, &err) + if ok { + return err.Header, nil + } + return nil, fmt.Errorf(fmt.Sprintf("Origin error: (%s) is not of type *http.Error.\n", e.origin.Error())) +} + +// GetHeaderValues returns the values from a Header key. If original error is not http.Error return standard error. +func (e *Error) GetHeaderValues(key string) ([]string, error) { + var err *http.Error + if errors.As(e.origin, &err) { + return err.Header.Values(textproto.CanonicalMIMEHeaderKey(key)), nil + } + return nil, fmt.Errorf(fmt.Sprintf("Origin error: (%s) is not of type http.Header.\n", e.origin.Error())) +} + +// GetHeader returns the first value from a header key. If origin is not http.Error or if no match is found returns "". +func (e *Error) GetHeader(key string) string { + var err *http.Error + if errors.As(e.origin, &err) { + return err.Header.Get(textproto.CanonicalMIMEHeaderKey(key)) + } + return "" +} diff --git a/api/write/error_test.go b/api/write/error_test.go new file mode 100644 index 00000000..6d433532 --- /dev/null +++ b/api/write/error_test.go @@ -0,0 +1,66 @@ +// Copyright 2020-2024 InfluxData, Inc. All rights reserved. +// Use of this source code is governed by MIT +// license that can be found in the LICENSE file. + +package write + +import ( + "errors" + "fmt" + "github.com/influxdata/influxdb-client-go/v2/api/http" + "github.com/stretchr/testify/assert" + ihttp "net/http" + "testing" +) + +func TestNewErrorNotHttpError(t *testing.T) { + err := NewError(fmt.Errorf("origin error"), "error message") + var errTest *http.Error + assert.False(t, errors.As(err, &errTest)) + header, okh := err.HTTPHeader() + assert.Nil(t, header) + assert.Error(t, okh) + values, okv := err.GetHeaderValues("Date") + assert.Nil(t, values) + assert.Error(t, okv) + assert.Equal(t, "", err.GetHeader("Date")) + assert.Equal(t, "error message:\n origin error", err.Error()) +} + +func TestNewErrorHttpError(t *testing.T) { + header := ihttp.Header{ + "Date": []string{"2024-08-07T12:00:00.009"}, + "Content-Length": []string{"12"}, + "Content-Type": []string{"application/json", "encoding UTF-8"}, + "X-Test-Value1": []string{"SaturnV"}, + "X-Test-Value2": []string{"Apollo11"}, + "Retry-After": []string{"2044"}, + "Trace-Id": []string{"123456789ABCDEF0"}, + } + + err := NewError(&http.Error{ + StatusCode: ihttp.StatusBadRequest, + Code: "bad request", + Message: "this is just a test", + Err: nil, + RetryAfter: 2044, + Header: header, + }, "should be httpError") + + var errTest *http.Error + assert.True(t, errors.As(err.Unwrap(), &errTest)) + header, okh := err.HTTPHeader() + assert.NotNil(t, header) + assert.Nil(t, okh) + date, okd := err.GetHeaderValues("Date") + assert.Equal(t, []string{"2024-08-07T12:00:00.009"}, date) + assert.Nil(t, okd) + cType, okc := err.GetHeaderValues("Content-Type") + assert.Equal(t, []string{"application/json", "encoding UTF-8"}, cType) + assert.Nil(t, okc) + assert.Equal(t, "2024-08-07T12:00:00.009", err.GetHeader("Date")) + assert.Equal(t, "SaturnV", err.GetHeader("X-Test-Value1")) + assert.Equal(t, "Apollo11", err.GetHeader("X-Test-Value2")) + assert.Equal(t, "123456789ABCDEF0", err.GetHeader("Trace-Id")) + assert.Equal(t, "should be httpError:\n bad request: this is just a test", err.Error()) +} diff --git a/api/write_test.go b/api/write_test.go index 4fda86bd..034a8f71 100644 --- a/api/write_test.go +++ b/api/write_test.go @@ -294,13 +294,15 @@ func TestWriteApiErrorHeaders(t *testing.T) { for i := 0; i < 3; i++ { recErr = <-errCh assert.NotNil(t, recErr, "errCh should not run out of values") - assert.Len(t, recErr.(*http.Error).Header, 6) - assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Date")) - assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Content-Length")) - assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Content-Type")) - assert.Equal(t, strconv.Itoa(i+1), recErr.(*http.Error).Header.Get("X-Call-Count")) - assert.Equal(t, "Not All Correct", recErr.(*http.Error).Header.Get("X-Test-Val1")) - assert.Equal(t, "Atlas LV-3B", recErr.(*http.Error).Header.Get("X-Test-Val2")) + header, okh := recErr.(*write.Error).HTTPHeader() + assert.Nil(t, okh) + assert.Len(t, header, 6) + assert.NotEqual(t, "", recErr.(*write.Error).GetHeader("Date")) + assert.NotEqual(t, "", recErr.(*write.Error).GetHeader("Content-Length")) + assert.NotEqual(t, "", recErr.(*write.Error).GetHeader("Content-Type")) + assert.Equal(t, strconv.Itoa(i+1), recErr.(*write.Error).GetHeader("X-Call-Count")) + assert.Equal(t, "Not All Correct", recErr.(*write.Error).GetHeader("X-Test-Val1")) + assert.Equal(t, "Atlas LV-3B", recErr.(*write.Error).GetHeader("X-Test-Val2")) } wg.Done() }() diff --git a/internal/write/service.go b/internal/write/service.go index e39acffb..4aa9b2ed 100644 --- a/internal/write/service.go +++ b/internal/write/service.go @@ -210,16 +210,7 @@ func (w *Service) HandleWrite(ctx context.Context, batch *Batch) error { } log.Error(logMessage) } - return &http2.Error{ - StatusCode: int(perror.StatusCode), - Code: perror.Code, - Message: fmt.Errorf( - "write failed (attempts %d): %w", batchToWrite.RetryAttempts, perror, - ).Error(), - Err: perror.Err, - RetryAfter: perror.RetryAfter, - Header: perror.Header, - } + return write.NewError(perror, fmt.Sprintf("write failed (retry attempts %d)", batchToWrite.RetryAttempts)) } } diff --git a/internal/write/service_test.go b/internal/write/service_test.go index ed0b077e..8638c82f 100644 --- a/internal/write/service_test.go +++ b/internal/write/service_test.go @@ -339,7 +339,7 @@ func TestMaxRetryTime(t *testing.T) { err = srv.HandleWrite(ctx, b) require.NotNil(t, err) // 1st Batch expires and writing 2nd trows error - assert.Equal(t, "write failed (attempts 1): Unexpected status code 429", err.Error()) + assert.Equal(t, "write failed (retry attempts 1):\n Unexpected status code 429", err.Error()) assert.Equal(t, 1, srv.retryQueue.list.Len()) //wait until remaining accumulated retryDelay has passed, because there hasn't been a successful write yet @@ -715,7 +715,7 @@ func TestHttpErrorHeaders(t *testing.T) { write.DefaultOptions()) err := svc.HandleWrite(context.Background(), NewBatch("1", 20)) assert.Error(t, err) - assert.Equal(t, "400 Bad Request: write failed (attempts 0): 400 Bad Request: { \"code\": \"bad request\", \"message\": \"test header\" }", err.Error()) - assert.Equal(t, "Not All Correct", err.(*http.Error).Header.Get("X-Test-Val1")) - assert.Equal(t, "Atlas LV-3B", err.(*http.Error).Header.Get("X-Test-Val2")) + assert.Equal(t, "write failed (retry attempts 0):\n 400 Bad Request: { \"code\": \"bad request\", \"message\": \"test header\" }", err.Error()) + assert.Equal(t, "Not All Correct", err.(*write.Error).GetHeader("X-Test-Val1")) + assert.Equal(t, "Atlas LV-3B", err.(*write.Error).GetHeader("X-Test-Val2")) } From 20a74f5a248c844dc2bc9f8e7de5502da022c9c4 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Thu, 8 Aug 2024 15:37:00 +0200 Subject: [PATCH 09/13] chore: remove case used in investigating http.Error and add test of http.Error.Error() response. --- api/http/error.go | 2 -- api/http/error_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/api/http/error.go b/api/http/error.go index 53a91a0b..af1a2dee 100644 --- a/api/http/error.go +++ b/api/http/error.go @@ -27,8 +27,6 @@ func (e *Error) Error() string { return e.Err.Error() case e.Code != "" && e.Message != "": return fmt.Sprintf("%s: %s", e.Code, e.Message) - case e.Message != "": - return e.Message default: return "Unexpected status code " + strconv.Itoa(e.StatusCode) } diff --git a/api/http/error_test.go b/api/http/error_test.go index ec5dc180..235d865f 100644 --- a/api/http/error_test.go +++ b/api/http/error_test.go @@ -5,6 +5,7 @@ package http import ( + "fmt" "github.com/stretchr/testify/assert" ihttp "net/http" @@ -51,3 +52,30 @@ func TestWriteErrorHeaderToString(t *testing.T) { assert.NotContains(t, filterString, "Content-Type: application/json") assert.NotContains(t, filterString, "Retry-After: 2044") } + +func TestErrorIfaceError(t *testing.T) { + tests := []struct { + statusCode int + err error + code string + message string + expected string + }{ + {statusCode: 418, err: fmt.Errorf("original test message"), code: "", message: "", expected: "original test message"}, + {statusCode: 418, err: fmt.Errorf("original test message"), code: "bad request", message: "is this a teapot?", expected: "original test message"}, + {statusCode: 418, err: nil, code: "bad request", message: "is this a teapot?", expected: "bad request: is this a teapot?"}, + {statusCode: 418, err: nil, code: "I'm a teapot", message: "", expected: "Unexpected status code 418"}, + } + + for _, test := range tests { + err := Error{ + StatusCode: test.statusCode, + Code: test.code, + Message: test.message, + Err: test.err, + RetryAfter: 0, + Header: ihttp.Header{}, + } + assert.Equal(t, test.expected, err.Error()) + } +} From 580d1eaaeffd585494a04a3a9ea53543335b270d Mon Sep 17 00:00:00 2001 From: karel rehor Date: Fri, 9 Aug 2024 10:10:29 +0200 Subject: [PATCH 10/13] docs: update ExampleWriteAPI_errors --- api/examples_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/examples_test.go b/api/examples_test.go index 01853c6d..edbff240 100644 --- a/api/examples_test.go +++ b/api/examples_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/influxdata/influxdb-client-go/v2/api" - apiHttp "github.com/influxdata/influxdb-client-go/v2/api/http" "github.com/influxdata/influxdb-client-go/v2/api/write" "github.com/influxdata/influxdb-client-go/v2/domain" influxdb2 "github.com/influxdata/influxdb-client-go/v2/internal/examples" @@ -124,7 +123,7 @@ func ExampleWriteAPI_errors() { go func() { for err := range errorsCh { fmt.Printf("write error: %s\n", err.Error()) - fmt.Printf("trace-id: %s\n", err.(*apiHttp.Error).Header.Get("Trace-ID")) + fmt.Printf("trace-id: %s\n", err.(*write.Error).GetHeader("Trace-ID")) } }() // write some points From 584c614db4c02e5c195f4535a8c896b11dae9be6 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Fri, 9 Aug 2024 14:52:14 +0200 Subject: [PATCH 11/13] chore: refactor - rm write/Error, exec goimports, simplify return from Service.HandleWrite --- api/examples_test.go | 3 +- api/write/error.go | 64 --------------------------------- api/write/error_test.go | 66 ---------------------------------- api/write_test.go | 16 ++++----- internal/write/service.go | 5 ++- internal/write/service_test.go | 8 ++--- 6 files changed, 17 insertions(+), 145 deletions(-) delete mode 100644 api/write/error.go delete mode 100644 api/write/error_test.go diff --git a/api/examples_test.go b/api/examples_test.go index edbff240..01853c6d 100644 --- a/api/examples_test.go +++ b/api/examples_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/influxdata/influxdb-client-go/v2/api" + apiHttp "github.com/influxdata/influxdb-client-go/v2/api/http" "github.com/influxdata/influxdb-client-go/v2/api/write" "github.com/influxdata/influxdb-client-go/v2/domain" influxdb2 "github.com/influxdata/influxdb-client-go/v2/internal/examples" @@ -123,7 +124,7 @@ func ExampleWriteAPI_errors() { go func() { for err := range errorsCh { fmt.Printf("write error: %s\n", err.Error()) - fmt.Printf("trace-id: %s\n", err.(*write.Error).GetHeader("Trace-ID")) + fmt.Printf("trace-id: %s\n", err.(*apiHttp.Error).Header.Get("Trace-ID")) } }() // write some points diff --git a/api/write/error.go b/api/write/error.go deleted file mode 100644 index f8c11865..00000000 --- a/api/write/error.go +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2020-2024 InfluxData, Inc. All rights reserved. -// Use of this source code is governed by MIT -// license that can be found in the LICENSE file. - -package write - -import ( - "errors" - "fmt" - "github.com/influxdata/influxdb-client-go/v2/api/http" - iHttp "net/http" - "net/textproto" -) - -// Error wraps an error that may have occurred during a write call. Most often this will be an http.Error. -type Error struct { - origin error - message string -} - -// NewError returns a new created Error instance wrapping the original error. -func NewError(origin error, message string) *Error { - return &Error{ - origin: origin, - message: message, - } -} - -// Error fulfills the error interface -func (e *Error) Error() string { - return fmt.Sprintf("%s:\n %s", e.message, e.origin) -} - -func (e *Error) Unwrap() error { - return e.origin -} - -// HTTPHeader returns the Header of a wrapped http.Error. If the original error is not http.Error returns standard error. -func (e *Error) HTTPHeader() (iHttp.Header, error) { - var err *http.Error - ok := errors.As(e.origin, &err) - if ok { - return err.Header, nil - } - return nil, fmt.Errorf(fmt.Sprintf("Origin error: (%s) is not of type *http.Error.\n", e.origin.Error())) -} - -// GetHeaderValues returns the values from a Header key. If original error is not http.Error return standard error. -func (e *Error) GetHeaderValues(key string) ([]string, error) { - var err *http.Error - if errors.As(e.origin, &err) { - return err.Header.Values(textproto.CanonicalMIMEHeaderKey(key)), nil - } - return nil, fmt.Errorf(fmt.Sprintf("Origin error: (%s) is not of type http.Header.\n", e.origin.Error())) -} - -// GetHeader returns the first value from a header key. If origin is not http.Error or if no match is found returns "". -func (e *Error) GetHeader(key string) string { - var err *http.Error - if errors.As(e.origin, &err) { - return err.Header.Get(textproto.CanonicalMIMEHeaderKey(key)) - } - return "" -} diff --git a/api/write/error_test.go b/api/write/error_test.go deleted file mode 100644 index 6d433532..00000000 --- a/api/write/error_test.go +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2020-2024 InfluxData, Inc. All rights reserved. -// Use of this source code is governed by MIT -// license that can be found in the LICENSE file. - -package write - -import ( - "errors" - "fmt" - "github.com/influxdata/influxdb-client-go/v2/api/http" - "github.com/stretchr/testify/assert" - ihttp "net/http" - "testing" -) - -func TestNewErrorNotHttpError(t *testing.T) { - err := NewError(fmt.Errorf("origin error"), "error message") - var errTest *http.Error - assert.False(t, errors.As(err, &errTest)) - header, okh := err.HTTPHeader() - assert.Nil(t, header) - assert.Error(t, okh) - values, okv := err.GetHeaderValues("Date") - assert.Nil(t, values) - assert.Error(t, okv) - assert.Equal(t, "", err.GetHeader("Date")) - assert.Equal(t, "error message:\n origin error", err.Error()) -} - -func TestNewErrorHttpError(t *testing.T) { - header := ihttp.Header{ - "Date": []string{"2024-08-07T12:00:00.009"}, - "Content-Length": []string{"12"}, - "Content-Type": []string{"application/json", "encoding UTF-8"}, - "X-Test-Value1": []string{"SaturnV"}, - "X-Test-Value2": []string{"Apollo11"}, - "Retry-After": []string{"2044"}, - "Trace-Id": []string{"123456789ABCDEF0"}, - } - - err := NewError(&http.Error{ - StatusCode: ihttp.StatusBadRequest, - Code: "bad request", - Message: "this is just a test", - Err: nil, - RetryAfter: 2044, - Header: header, - }, "should be httpError") - - var errTest *http.Error - assert.True(t, errors.As(err.Unwrap(), &errTest)) - header, okh := err.HTTPHeader() - assert.NotNil(t, header) - assert.Nil(t, okh) - date, okd := err.GetHeaderValues("Date") - assert.Equal(t, []string{"2024-08-07T12:00:00.009"}, date) - assert.Nil(t, okd) - cType, okc := err.GetHeaderValues("Content-Type") - assert.Equal(t, []string{"application/json", "encoding UTF-8"}, cType) - assert.Nil(t, okc) - assert.Equal(t, "2024-08-07T12:00:00.009", err.GetHeader("Date")) - assert.Equal(t, "SaturnV", err.GetHeader("X-Test-Value1")) - assert.Equal(t, "Apollo11", err.GetHeader("X-Test-Value2")) - assert.Equal(t, "123456789ABCDEF0", err.GetHeader("Trace-Id")) - assert.Equal(t, "should be httpError:\n bad request: this is just a test", err.Error()) -} diff --git a/api/write_test.go b/api/write_test.go index 034a8f71..4fda86bd 100644 --- a/api/write_test.go +++ b/api/write_test.go @@ -294,15 +294,13 @@ func TestWriteApiErrorHeaders(t *testing.T) { for i := 0; i < 3; i++ { recErr = <-errCh assert.NotNil(t, recErr, "errCh should not run out of values") - header, okh := recErr.(*write.Error).HTTPHeader() - assert.Nil(t, okh) - assert.Len(t, header, 6) - assert.NotEqual(t, "", recErr.(*write.Error).GetHeader("Date")) - assert.NotEqual(t, "", recErr.(*write.Error).GetHeader("Content-Length")) - assert.NotEqual(t, "", recErr.(*write.Error).GetHeader("Content-Type")) - assert.Equal(t, strconv.Itoa(i+1), recErr.(*write.Error).GetHeader("X-Call-Count")) - assert.Equal(t, "Not All Correct", recErr.(*write.Error).GetHeader("X-Test-Val1")) - assert.Equal(t, "Atlas LV-3B", recErr.(*write.Error).GetHeader("X-Test-Val2")) + assert.Len(t, recErr.(*http.Error).Header, 6) + assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Date")) + assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Content-Length")) + assert.NotEqual(t, "", recErr.(*http.Error).Header.Get("Content-Type")) + assert.Equal(t, strconv.Itoa(i+1), recErr.(*http.Error).Header.Get("X-Call-Count")) + assert.Equal(t, "Not All Correct", recErr.(*http.Error).Header.Get("X-Test-Val1")) + assert.Equal(t, "Atlas LV-3B", recErr.(*http.Error).Header.Get("X-Test-Val2")) } wg.Done() }() diff --git a/internal/write/service.go b/internal/write/service.go index 4aa9b2ed..44ddf1c1 100644 --- a/internal/write/service.go +++ b/internal/write/service.go @@ -210,7 +210,10 @@ func (w *Service) HandleWrite(ctx context.Context, batch *Batch) error { } log.Error(logMessage) } - return write.NewError(perror, fmt.Sprintf("write failed (retry attempts %d)", batchToWrite.RetryAttempts)) + log.Errorf("Write failed (retry attempts %d): Status Code %d", + batchToWrite.RetryAttempts, + perror.StatusCode) + return perror } } diff --git a/internal/write/service_test.go b/internal/write/service_test.go index 8638c82f..d751339a 100644 --- a/internal/write/service_test.go +++ b/internal/write/service_test.go @@ -339,7 +339,7 @@ func TestMaxRetryTime(t *testing.T) { err = srv.HandleWrite(ctx, b) require.NotNil(t, err) // 1st Batch expires and writing 2nd trows error - assert.Equal(t, "write failed (retry attempts 1):\n Unexpected status code 429", err.Error()) + assert.Equal(t, "Unexpected status code 429", err.Error()) assert.Equal(t, 1, srv.retryQueue.list.Len()) //wait until remaining accumulated retryDelay has passed, because there hasn't been a successful write yet @@ -715,7 +715,7 @@ func TestHttpErrorHeaders(t *testing.T) { write.DefaultOptions()) err := svc.HandleWrite(context.Background(), NewBatch("1", 20)) assert.Error(t, err) - assert.Equal(t, "write failed (retry attempts 0):\n 400 Bad Request: { \"code\": \"bad request\", \"message\": \"test header\" }", err.Error()) - assert.Equal(t, "Not All Correct", err.(*write.Error).GetHeader("X-Test-Val1")) - assert.Equal(t, "Atlas LV-3B", err.(*write.Error).GetHeader("X-Test-Val2")) + assert.Equal(t, "400 Bad Request: { \"code\": \"bad request\", \"message\": \"test header\" }", err.Error()) + assert.Equal(t, "Not All Correct", err.(*http.Error).Header.Get("X-Test-Val1")) + assert.Equal(t, "Atlas LV-3B", err.(*http.Error).Header.Get("X-Test-Val2")) } From 40eaebb9dd6b4a966a2a8247a286913924167ca0 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Fri, 9 Aug 2024 15:19:59 +0200 Subject: [PATCH 12/13] test: use t.Run for data driven test, exec goimports --- api/http/error_test.go | 52 ++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/api/http/error_test.go b/api/http/error_test.go index 235d865f..ba8455db 100644 --- a/api/http/error_test.go +++ b/api/http/error_test.go @@ -6,9 +6,10 @@ package http import ( "fmt" - "github.com/stretchr/testify/assert" ihttp "net/http" + "github.com/stretchr/testify/assert" + "testing" ) @@ -55,27 +56,50 @@ func TestWriteErrorHeaderToString(t *testing.T) { func TestErrorIfaceError(t *testing.T) { tests := []struct { + name string statusCode int err error code string message string expected string }{ - {statusCode: 418, err: fmt.Errorf("original test message"), code: "", message: "", expected: "original test message"}, - {statusCode: 418, err: fmt.Errorf("original test message"), code: "bad request", message: "is this a teapot?", expected: "original test message"}, - {statusCode: 418, err: nil, code: "bad request", message: "is this a teapot?", expected: "bad request: is this a teapot?"}, - {statusCode: 418, err: nil, code: "I'm a teapot", message: "", expected: "Unexpected status code 418"}, + {name: "TestNestedErrorNotNilCode0Message0", + statusCode: 418, + err: fmt.Errorf("original test message"), + code: "", + message: "", + expected: "original test message"}, + {name: "TestNestedErrorNotNilCodeXMessageX", + statusCode: 418, + err: fmt.Errorf("original test message"), + code: "bad request", + message: "is this a teapot?", + expected: "original test message"}, + {name: "TestNestedErrorNilCodeXMessageX", + statusCode: 418, + err: nil, + code: "bad request", + message: "is this a teapot?", + expected: "bad request: is this a teapot?"}, + {name: "TestNesterErrorNilCodeXMessage0", + statusCode: 418, + err: nil, + code: "I'm a teapot", + message: "", + expected: "Unexpected status code 418"}, } for _, test := range tests { - err := Error{ - StatusCode: test.statusCode, - Code: test.code, - Message: test.message, - Err: test.err, - RetryAfter: 0, - Header: ihttp.Header{}, - } - assert.Equal(t, test.expected, err.Error()) + t.Run(test.name, func(t *testing.T) { + err := Error{ + StatusCode: test.statusCode, + Code: test.code, + Message: test.message, + Err: test.err, + RetryAfter: 0, + Header: ihttp.Header{}, + } + assert.Equal(t, test.expected, err.Error()) + }) } } From 5858f61e981ccf531a28b55f9459e2b86166a8f8 Mon Sep 17 00:00:00 2001 From: karel rehor Date: Fri, 9 Aug 2024 16:45:51 +0200 Subject: [PATCH 13/13] docs: update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f95e670a..55c5704f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ### Features - [#404](https://github.com/influxdata/influxdb-client-go/pull/404) Expose HTTP response headers in the Error type to aid analysis and debugging of error results. Add selected response headers to the error log. + + Also, unified errors returned by WriteAPI, which now always returns `http.Error` ### Fixes - [#403](https://github.com/influxdata/influxdb-client-go/pull/403) Custom checks de/serialization to allow calling server Check API