From 88bf1b1a68e42696f8eb1903e54c9971166c517e Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 16 Apr 2025 03:57:42 +0800 Subject: [PATCH 1/4] Allow unwrapping of errors when reading from remote client Signed-off-by: Jeanette Tan --- storage/remote/client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/remote/client.go b/storage/remote/client.go index f00b3e7331..af696336bd 100644 --- a/storage/remote/client.go +++ b/storage/remote/client.go @@ -384,7 +384,8 @@ func (c *Client) Read(ctx context.Context, query *prompb.Query, sortSeries bool) _ = httpResp.Body.Close() cancel() - return nil, fmt.Errorf("remote server %s returned http status %s: %s", c.urlString, httpResp.Status, string(body)) + err := errors.New(string(body)) + return nil, fmt.Errorf("remote server %s returned http status %s: %w", c.urlString, httpResp.Status, err) } contentType := httpResp.Header.Get("Content-Type") From f1b66948374097b53c25d45c2c16aa9a145ddfe9 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 16 Apr 2025 03:57:42 +0800 Subject: [PATCH 2/4] Add unit test Signed-off-by: Jeanette Tan --- storage/remote/client_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/storage/remote/client_test.go b/storage/remote/client_test.go index 112e96d2b6..2ed825d6d5 100644 --- a/storage/remote/client_test.go +++ b/storage/remote/client_test.go @@ -409,6 +409,34 @@ func TestReadClient(t *testing.T) { } } +func TestReadClientUnwrapError(t *testing.T) { + httpHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "test error", http.StatusBadRequest) + }) + expectedError := "test error\n" + + server := httptest.NewServer(httpHandler) + defer server.Close() + + u, err := url.Parse(server.URL) + require.NoError(t, err) + + conf := &ClientConfig{ + URL: &config_util.URL{URL: u}, + Timeout: model.Duration(5 * time.Second), + ChunkedReadLimit: config.DefaultChunkedReadLimit, + } + c, err := NewReadClient("test", conf) + require.NoError(t, err) + + query := &prompb.Query{} + + _, err = c.Read(context.Background(), query, false) + require.ErrorContains(t, err, expectedError) + err = errors.Unwrap(err) + require.EqualError(t, err, expectedError) +} + func sampledResponseHTTPHandler(t *testing.T) http.HandlerFunc { return func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/x-protobuf") From ab2d17d7a01e9ebe7eb2b655c2ec3921c76143d8 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 16 Apr 2025 03:57:42 +0800 Subject: [PATCH 3/4] Move unit test to existing suite Signed-off-by: Jeanette Tan --- storage/remote/client_test.go | 41 +++++++++++------------------------ 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/storage/remote/client_test.go b/storage/remote/client_test.go index 2ed825d6d5..39e0dd9402 100644 --- a/storage/remote/client_test.go +++ b/storage/remote/client_test.go @@ -225,6 +225,7 @@ func TestReadClient(t *testing.T) { expectedSamples [][]model.SamplePair expectedErrorContains string sortSeries bool + unwrap bool }{ { name: "sorted sampled response", @@ -336,6 +337,14 @@ func TestReadClient(t *testing.T) { timeout: 5 * time.Millisecond, expectedErrorContains: "context deadline exceeded: request timed out after 5ms", }, + { + name: "unwrap error", + httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "test error", http.StatusBadRequest) + }), + expectedErrorContains: "test error", + unwrap: true, + }, } for _, test := range tests { @@ -366,6 +375,10 @@ func TestReadClient(t *testing.T) { ss, err := c.Read(context.Background(), query, test.sortSeries) if test.expectedErrorContains != "" { require.ErrorContains(t, err, test.expectedErrorContains) + if test.unwrap { + err = errors.Unwrap(err) + require.EqualError(t, err, test.expectedErrorContains+"\n") + } return } @@ -409,34 +422,6 @@ func TestReadClient(t *testing.T) { } } -func TestReadClientUnwrapError(t *testing.T) { - httpHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - http.Error(w, "test error", http.StatusBadRequest) - }) - expectedError := "test error\n" - - server := httptest.NewServer(httpHandler) - defer server.Close() - - u, err := url.Parse(server.URL) - require.NoError(t, err) - - conf := &ClientConfig{ - URL: &config_util.URL{URL: u}, - Timeout: model.Duration(5 * time.Second), - ChunkedReadLimit: config.DefaultChunkedReadLimit, - } - c, err := NewReadClient("test", conf) - require.NoError(t, err) - - query := &prompb.Query{} - - _, err = c.Read(context.Background(), query, false) - require.ErrorContains(t, err, expectedError) - err = errors.Unwrap(err) - require.EqualError(t, err, expectedError) -} - func sampledResponseHTTPHandler(t *testing.T) http.HandlerFunc { return func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/x-protobuf") From b91c66cdf980862d33d6d5d124782a49beb25ae4 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 16 Apr 2025 17:58:19 +0800 Subject: [PATCH 4/4] Move newline to inside the expected error Signed-off-by: Jeanette Tan --- storage/remote/client_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/remote/client_test.go b/storage/remote/client_test.go index 39e0dd9402..27a322c39f 100644 --- a/storage/remote/client_test.go +++ b/storage/remote/client_test.go @@ -342,7 +342,7 @@ func TestReadClient(t *testing.T) { httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { http.Error(w, "test error", http.StatusBadRequest) }), - expectedErrorContains: "test error", + expectedErrorContains: "test error\n", unwrap: true, }, } @@ -377,7 +377,7 @@ func TestReadClient(t *testing.T) { require.ErrorContains(t, err, test.expectedErrorContains) if test.unwrap { err = errors.Unwrap(err) - require.EqualError(t, err, test.expectedErrorContains+"\n") + require.EqualError(t, err, test.expectedErrorContains) } return }