Skip to content

Commit 38bdc81

Browse files
eiixyeiixy
authored
Optimize Client Error Return (#856)
* update client error return * update client_test.go * update client_test.go * update file_api_test.go * update client_test.go * update client_test.go
1 parent e095df5 commit 38bdc81

File tree

4 files changed

+67
-25
lines changed

4 files changed

+67
-25
lines changed

‎client.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,18 @@ func (c *Client) baseURLWithAzureDeployment(baseURL, suffix, model string) (newB
285285
}
286286

287287
func (c *Client) handleErrorResp(resp *http.Response) error {
288+
if !strings.HasPrefix(resp.Header.Get("Content-Type"), "application/json") {
289+
body, err := io.ReadAll(resp.Body)
290+
if err != nil {
291+
return fmt.Errorf("error, reading response body: %w", err)
292+
}
293+
return fmt.Errorf("error, status code: %d, status: %s, body: %s", resp.StatusCode, resp.Status, body)
294+
}
288295
var errRes ErrorResponse
289296
err := json.NewDecoder(resp.Body).Decode(&errRes)
290297
if err != nil || errRes.Error == nil {
291298
reqErr := &RequestError{
299+
HTTPStatus: resp.Status,
292300
HTTPStatusCode: resp.StatusCode,
293301
Err: err,
294302
}
@@ -298,6 +306,7 @@ func (c *Client) handleErrorResp(resp *http.Response) error {
298306
return reqErr
299307
}
300308

309+
errRes.Error.HTTPStatus = resp.Status
301310
errRes.Error.HTTPStatusCode = resp.StatusCode
302311
return errRes.Error
303312
}

‎client_test.go‎

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,17 @@ func TestHandleErrorResp(t *testing.T) {
134134
client := NewClient(mockToken)
135135

136136
testCases := []struct {
137-
name string
138-
httpCode int
139-
body io.Reader
140-
expected string
137+
name string
138+
httpCode int
139+
httpStatus string
140+
contentType string
141+
body io.Reader
142+
expected string
141143
}{
142144
{
143-
name: "401 Invalid Authentication",
144-
httpCode: http.StatusUnauthorized,
145+
name: "401 Invalid Authentication",
146+
httpCode: http.StatusUnauthorized,
147+
contentType: "application/json",
145148
body: bytes.NewReader([]byte(
146149
`{
147150
"error":{
@@ -152,11 +155,12 @@ func TestHandleErrorResp(t *testing.T) {
152155
}
153156
}`,
154157
)),
155-
expected: "error, status code: 401, message: You didn't provide an API key. ....",
158+
expected: "error, status code: 401, status: , message: You didn't provide an API key. ....",
156159
},
157160
{
158-
name: "401 Azure Access Denied",
159-
httpCode: http.StatusUnauthorized,
161+
name: "401 Azure Access Denied",
162+
httpCode: http.StatusUnauthorized,
163+
contentType: "application/json",
160164
body: bytes.NewReader([]byte(
161165
`{
162166
"error":{
@@ -165,11 +169,12 @@ func TestHandleErrorResp(t *testing.T) {
165169
}
166170
}`,
167171
)),
168-
expected: "error, status code: 401, message: Access denied due to Virtual Network/Firewall rules.",
172+
expected: "error, status code: 401, status: , message: Access denied due to Virtual Network/Firewall rules.",
169173
},
170174
{
171-
name: "503 Model Overloaded",
172-
httpCode: http.StatusServiceUnavailable,
175+
name: "503 Model Overloaded",
176+
httpCode: http.StatusServiceUnavailable,
177+
contentType: "application/json",
173178
body: bytes.NewReader([]byte(`
174179
{
175180
"error":{
@@ -179,22 +184,53 @@ func TestHandleErrorResp(t *testing.T) {
179184
"code":null
180185
}
181186
}`)),
182-
expected: "error, status code: 503, message: That model...",
187+
expected: "error, status code: 503, status: , message: That model...",
183188
},
184189
{
185-
name: "503 no message (Unknown response)",
186-
httpCode: http.StatusServiceUnavailable,
190+
name: "503 no message (Unknown response)",
191+
httpCode: http.StatusServiceUnavailable,
192+
contentType: "application/json",
187193
body: bytes.NewReader([]byte(`
188194
{
189195
"error":{}
190196
}`)),
191-
expected: "error, status code: 503, message: ",
197+
expected: "error, status code: 503, status: , message: ",
198+
},
199+
{
200+
name: "413 Request Entity Too Large",
201+
httpCode: http.StatusRequestEntityTooLarge,
202+
contentType: "text/html",
203+
body: bytes.NewReader([]byte(`<html>
204+
<head><title>413 Request Entity Too Large</title></head>
205+
<body>
206+
<center><h1>413 Request Entity Too Large</h1></center>
207+
<hr><center>nginx</center>
208+
</body>
209+
</html>`)),
210+
expected: `error, status code: 413, status: , body: <html>
211+
<head><title>413 Request Entity Too Large</title></head>
212+
<body>
213+
<center><h1>413 Request Entity Too Large</h1></center>
214+
<hr><center>nginx</center>
215+
</body>
216+
</html>`,
217+
},
218+
{
219+
name: "errorReader",
220+
httpCode: http.StatusRequestEntityTooLarge,
221+
contentType: "text/html",
222+
body: &errorReader{err: errors.New("errorReader")},
223+
expected: "error, reading response body: errorReader",
192224
},
193225
}
194226

195227
for _, tc := range testCases {
196228
t.Run(tc.name, func(t *testing.T) {
197-
testCase := &http.Response{}
229+
testCase := &http.Response{
230+
Header: map[string][]string{
231+
"Content-Type": {tc.contentType},
232+
},
233+
}
198234
testCase.StatusCode = tc.httpCode
199235
testCase.Body = io.NopCloser(tc.body)
200236
err := client.handleErrorResp(testCase)
@@ -203,12 +239,6 @@ func TestHandleErrorResp(t *testing.T) {
203239
t.Errorf("Unexpected error: %v , expected: %s", err, tc.expected)
204240
t.Fail()
205241
}
206-
207-
e := &APIError{}
208-
if !errors.As(err, &e) {
209-
t.Errorf("(%s) Expected error to be of type APIError", tc.name)
210-
t.Fail()
211-
}
212242
})
213243
}
214244
}

‎error.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ type APIError struct {
1313
Message string `json:"message"`
1414
Param *string `json:"param,omitempty"`
1515
Type string `json:"type"`
16+
HTTPStatus string `json:"-"`
1617
HTTPStatusCode int `json:"-"`
1718
InnerError *InnerError `json:"innererror,omitempty"`
1819
}
@@ -25,6 +26,7 @@ type InnerError struct {
2526

2627
// RequestError provides information about generic request errors.
2728
type RequestError struct {
29+
HTTPStatus string
2830
HTTPStatusCode int
2931
Err error
3032
}
@@ -35,7 +37,7 @@ type ErrorResponse struct {
3537

3638
func (e *APIError) Error() string {
3739
if e.HTTPStatusCode > 0 {
38-
return fmt.Sprintf("error, status code: %d, message: %s", e.HTTPStatusCode, e.Message)
40+
return fmt.Sprintf("error, status code: %d, status: %s, message: %s", e.HTTPStatusCode, e.HTTPStatus, e.Message)
3941
}
4042

4143
return e.Message
@@ -101,7 +103,7 @@ func (e *APIError) UnmarshalJSON(data []byte) (err error) {
101103
}
102104

103105
func (e *RequestError) Error() string {
104-
return fmt.Sprintf("error, status code: %d, message: %s", e.HTTPStatusCode, e.Err)
106+
return fmt.Sprintf("error, status code: %d, status: %s, message: %s", e.HTTPStatusCode, e.HTTPStatus, e.Err)
105107
}
106108

107109
func (e *RequestError) Unwrap() error {

‎files_api_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ func TestGetFileContentReturnError(t *testing.T) {
152152
client, server, teardown := setupOpenAITestServer()
153153
defer teardown()
154154
server.RegisterHandler("/v1/files/deadbeef/content", func(w http.ResponseWriter, _ *http.Request) {
155+
w.Header().Set("Content-Type", "application/json")
155156
w.WriteHeader(http.StatusBadRequest)
156157
fmt.Fprint(w, wantErrorResp)
157158
})

0 commit comments

Comments
 (0)