Skip to content

Commit f8df5a7

Browse files
metonymmetonym
authored
Format error messages for object storage (EA) (#256)
Improves error message formatting for object storage presigned URL operations by returning user-friendly messages instead of raw provider responses. The format follows the existing `received response code %d: %s`.
1 parent 78884e7 commit f8df5a7

File tree

2 files changed

+281
-4
lines changed

2 files changed

+281
-4
lines changed

‎pkg/storage/repo.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package storage
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"io"
78
"net/http"
@@ -147,8 +148,7 @@ func (r *Repo) UploadToPresignedURL(ctx context.Context, presignedURL string, co
147148
defer resp.Body.Close()
148149

149150
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
150-
respBody, _ := io.ReadAll(resp.Body)
151-
return fmt.Errorf("upload failed with status %d: %s", resp.StatusCode, string(respBody))
151+
return errors.New(storageErrorMessage(resp.StatusCode))
152152
}
153153

154154
return nil
@@ -168,8 +168,7 @@ func (r *Repo) DownloadFromPresignedURL(ctx context.Context, presignedURL string
168168
defer resp.Body.Close()
169169

170170
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
171-
respBody, _ := io.ReadAll(resp.Body)
172-
return 0, fmt.Errorf("download failed with status %d: %s", resp.StatusCode, string(respBody))
171+
return 0, errors.New(storageErrorMessage(resp.StatusCode))
173172
}
174173

175174
written, err := io.Copy(dest, resp.Body)
@@ -179,3 +178,26 @@ func (r *Repo) DownloadFromPresignedURL(ctx context.Context, presignedURL string
179178

180179
return written, nil
181180
}
181+
182+
func storageErrorMessage(statusCode int) string {
183+
var message string
184+
switch statusCode {
185+
case 400:
186+
message = "bad request"
187+
case 401, 403:
188+
message = "access denied"
189+
case 404:
190+
message = "object not found"
191+
case 409:
192+
message = "conflict"
193+
case 413:
194+
message = "object too large"
195+
case 429:
196+
message = "rate limited, please try again later"
197+
case 500, 502, 503, 504:
198+
message = "storage service temporarily unavailable"
199+
default:
200+
message = "unexpected error"
201+
}
202+
return fmt.Sprintf("received response code %d: %s", statusCode, message)
203+
}

‎pkg/storage/repo_test.go‎

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
package storage
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"net/http"
7+
"net/http/httptest"
8+
"strings"
9+
"testing"
10+
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestStorageErrorMessage(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
statusCode int
18+
want string
19+
}{
20+
{
21+
name: "400 bad request",
22+
statusCode: 400,
23+
want: "received response code 400: bad request",
24+
},
25+
{
26+
name: "401 unauthorized",
27+
statusCode: 401,
28+
want: "received response code 401: access denied",
29+
},
30+
{
31+
name: "403 forbidden",
32+
statusCode: 403,
33+
want: "received response code 403: access denied",
34+
},
35+
{
36+
name: "404 not found",
37+
statusCode: 404,
38+
want: "received response code 404: object not found",
39+
},
40+
{
41+
name: "409 conflict",
42+
statusCode: 409,
43+
want: "received response code 409: conflict",
44+
},
45+
{
46+
name: "413 payload too large",
47+
statusCode: 413,
48+
want: "received response code 413: object too large",
49+
},
50+
{
51+
name: "429 too many requests",
52+
statusCode: 429,
53+
want: "received response code 429: rate limited, please try again later",
54+
},
55+
{
56+
name: "500 internal server error",
57+
statusCode: 500,
58+
want: "received response code 500: storage service temporarily unavailable",
59+
},
60+
{
61+
name: "502 bad gateway",
62+
statusCode: 502,
63+
want: "received response code 502: storage service temporarily unavailable",
64+
},
65+
{
66+
name: "503 service unavailable",
67+
statusCode: 503,
68+
want: "received response code 503: storage service temporarily unavailable",
69+
},
70+
{
71+
name: "504 gateway timeout",
72+
statusCode: 504,
73+
want: "received response code 504: storage service temporarily unavailable",
74+
},
75+
{
76+
name: "unknown status code",
77+
statusCode: 418,
78+
want: "received response code 418: unexpected error",
79+
},
80+
}
81+
82+
for _, tt := range tests {
83+
t.Run(tt.name, func(t *testing.T) {
84+
got := storageErrorMessage(tt.statusCode)
85+
require.Equal(t, tt.want, got)
86+
})
87+
}
88+
}
89+
90+
func TestDownloadFromPresignedURL_StorageErrors(t *testing.T) {
91+
tests := []struct {
92+
name string
93+
statusCode int
94+
responseBody string
95+
wantErrContain []string
96+
wantErrExclude []string
97+
}{
98+
{
99+
name: "404 with S3 XML error",
100+
statusCode: 404,
101+
responseBody: `<?xml version="1.0" encoding="UTF-8"?>
102+
<Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message><Key>tea-d3tc3fuuk2gs73d0paug/foo/bar/test.txt</Key><RequestId>95N55HR7H0QBF3X9</RequestId><HostId>QfLXA55SGkqZ6VEKV97lgMjiFNWRFhpTj29FAylq2SOh2LJFMyvHuRdUjDu1IaZ/NmQR0znt4/0=</HostId></Error>`,
103+
wantErrContain: []string{"received response code 404", "object not found"},
104+
wantErrExclude: []string{"NoSuchKey", "tea-d3tc3fuuk2gs73d0paug", "RequestId", "HostId"},
105+
},
106+
{
107+
name: "403 with GCS error",
108+
statusCode: 403,
109+
responseBody: `<?xml version='1.0' encoding='UTF-8'?>
110+
<Error><Code>AccessDenied</Code><Message>Access denied.</Message><Details>render-objects-bucket/some/path</Details></Error>`,
111+
wantErrContain: []string{"received response code 403", "access denied"},
112+
wantErrExclude: []string{"render-objects-bucket", "AccessDenied", "Details"},
113+
},
114+
{
115+
name: "500 server error",
116+
statusCode: 500,
117+
responseBody: `Internal Server Error: connection to storage backend failed`,
118+
wantErrContain: []string{"received response code 500", "storage service temporarily unavailable"},
119+
wantErrExclude: []string{"Internal Server Error", "storage backend"},
120+
},
121+
}
122+
123+
for _, tt := range tests {
124+
t.Run(tt.name, func(t *testing.T) {
125+
// Create mock server that returns the specified status and body
126+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
127+
w.WriteHeader(tt.statusCode)
128+
w.Write([]byte(tt.responseBody))
129+
}))
130+
defer server.Close()
131+
132+
repo := &Repo{
133+
httpClient: server.Client(),
134+
}
135+
136+
var buf bytes.Buffer
137+
_, err := repo.DownloadFromPresignedURL(context.Background(), server.URL, &buf)
138+
139+
require.Error(t, err)
140+
for _, expected := range tt.wantErrContain {
141+
require.Contains(t, err.Error(), expected)
142+
}
143+
144+
// Verify sensitive information is NOT exposed
145+
for _, excluded := range tt.wantErrExclude {
146+
require.NotContains(t, err.Error(), excluded,
147+
"error message should not contain sensitive info: %s", excluded)
148+
}
149+
})
150+
}
151+
}
152+
153+
func TestUploadToPresignedURL_StorageErrors(t *testing.T) {
154+
tests := []struct {
155+
name string
156+
statusCode int
157+
responseBody string
158+
wantErrContain []string
159+
wantErrExclude []string
160+
}{
161+
{
162+
name: "403 with S3 access denied",
163+
statusCode: 403,
164+
responseBody: `<?xml version="1.0" encoding="UTF-8"?>
165+
<Error><Code>AccessDenied</Code><Message>Access Denied</Message><RequestId>ABC123</RequestId><HostId>xyz789</HostId></Error>`,
166+
wantErrContain: []string{"received response code 403", "access denied"},
167+
wantErrExclude: []string{"AccessDenied", "RequestId", "HostId", "ABC123", "xyz789"},
168+
},
169+
{
170+
name: "413 payload too large",
171+
statusCode: 413,
172+
responseBody: `<?xml version="1.0" encoding="UTF-8"?>
173+
<Error><Code>EntityTooLarge</Code><Message>Your proposed upload exceeds the maximum allowed size</Message><MaxSizeAllowed>5368709120</MaxSizeAllowed></Error>`,
174+
wantErrContain: []string{"received response code 413", "object too large"},
175+
wantErrExclude: []string{"EntityTooLarge", "MaxSizeAllowed", "5368709120"},
176+
},
177+
{
178+
name: "502 bad gateway",
179+
statusCode: 502,
180+
responseBody: `Bad Gateway: upstream storage unavailable`,
181+
wantErrContain: []string{"received response code 502", "storage service temporarily unavailable"},
182+
wantErrExclude: []string{"Bad Gateway", "upstream"},
183+
},
184+
}
185+
186+
for _, tt := range tests {
187+
t.Run(tt.name, func(t *testing.T) {
188+
// Create mock server that returns the specified status and body
189+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
190+
w.WriteHeader(tt.statusCode)
191+
w.Write([]byte(tt.responseBody))
192+
}))
193+
defer server.Close()
194+
195+
repo := &Repo{
196+
httpClient: server.Client(),
197+
}
198+
199+
content := strings.NewReader("test content")
200+
err := repo.UploadToPresignedURL(context.Background(), server.URL, content, 12)
201+
202+
require.Error(t, err)
203+
for _, expected := range tt.wantErrContain {
204+
require.Contains(t, err.Error(), expected)
205+
}
206+
207+
// Verify sensitive information is NOT exposed
208+
for _, excluded := range tt.wantErrExclude {
209+
require.NotContains(t, err.Error(), excluded,
210+
"error message should not contain sensitive info: %s", excluded)
211+
}
212+
})
213+
}
214+
}
215+
216+
func TestDownloadFromPresignedURL_Success(t *testing.T) {
217+
expectedContent := "file content here"
218+
219+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
220+
w.WriteHeader(http.StatusOK)
221+
w.Write([]byte(expectedContent))
222+
}))
223+
defer server.Close()
224+
225+
repo := &Repo{
226+
httpClient: server.Client(),
227+
}
228+
229+
var buf bytes.Buffer
230+
written, err := repo.DownloadFromPresignedURL(context.Background(), server.URL, &buf)
231+
232+
require.NoError(t, err)
233+
require.Equal(t, int64(len(expectedContent)), written)
234+
require.Equal(t, expectedContent, buf.String())
235+
}
236+
237+
func TestUploadToPresignedURL_Success(t *testing.T) {
238+
var receivedContent bytes.Buffer
239+
240+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
241+
receivedContent.ReadFrom(r.Body)
242+
w.WriteHeader(http.StatusOK)
243+
}))
244+
defer server.Close()
245+
246+
repo := &Repo{
247+
httpClient: server.Client(),
248+
}
249+
250+
content := "test upload content"
251+
err := repo.UploadToPresignedURL(context.Background(), server.URL, strings.NewReader(content), int64(len(content)))
252+
253+
require.NoError(t, err)
254+
require.Equal(t, content, receivedContent.String())
255+
}

0 commit comments

Comments
 (0)