refactor: introduce HTTPError typed error in remote client
Add HTTPError struct so callers can use errors.As to check HTTP status codes instead of fragile string matching. parseErrorResponse now returns *HTTPError with StatusCode, Status, and Body fields while preserving the same Error() string format.
- commit
- 34cd6d6dc1e1cda6f7f38fc4d898174425ce01bb
- author
- brice · · 2026-03-19 04:51 UTC
- parent
- e205712
- signature
- unsigned
2 files · +63 -2
M internal/adapters/in/cli/remote/client.go +19 -2
| @@ -203,14 +203,31 @@ func parseResponse(resp *http.Response, target any) error { | |||
| 203 | 203 | return nil | |
| 204 | 204 | } | |
| 205 | 205 | ||
| 206 | + | // HTTPError represents an HTTP error response with a status code. | |
| 207 | + | // Use errors.As to check for specific status codes in error handling. | |
| 208 | + | type HTTPError struct { | |
| 209 | + | StatusCode int | |
| 210 | + | Status string | |
| 211 | + | Body string | |
| 212 | + | } | |
| 213 | + | ||
| 214 | + | func (e *HTTPError) Error() string { | |
| 215 | + | return e.Status + ": " + e.Body | |
| 216 | + | } | |
| 217 | + | ||
| 206 | 218 | func parseErrorResponse(resp *http.Response, body []byte) error { | |
| 219 | + | msg := string(body) | |
| 207 | 220 | var errResp struct { | |
| 208 | 221 | Error string `json:"error"` | |
| 209 | 222 | } | |
| 210 | 223 | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | |
| 211 | - | return fmt.Errorf("%s: %s", resp.Status, errResp.Error) | |
| 224 | + | msg = errResp.Error | |
| 225 | + | } | |
| 226 | + | return &HTTPError{ | |
| 227 | + | StatusCode: resp.StatusCode, | |
| 228 | + | Status: resp.Status, | |
| 229 | + | Body: msg, | |
| 212 | 230 | } | |
| 213 | - | return fmt.Errorf("%s: %s", resp.Status, string(body)) | |
| 214 | 231 | } | |
| 215 | 232 | ||
| 216 | 233 | func isRetryableRequestError(err error) bool { | |
| @@ -203,14 +203,31 @@ func parseResponse(resp *http.Response, target any) error { | |||||
| 203 | return nil | 203 | return nil | ||
| 204 | } | 204 | } | ||
| 205 | 205 | ||||
| 206 | + | // HTTPError represents an HTTP error response with a status code. | |||
| 207 | + | // Use errors.As to check for specific status codes in error handling. | |||
| 208 | + | type HTTPError struct { | |||
| 209 | + | StatusCode int | |||
| 210 | + | Status string | |||
| 211 | + | Body string | |||
| 212 | + | } | |||
| 213 | + | ||||
| 214 | + | func (e *HTTPError) Error() string { | |||
| 215 | + | return e.Status + ": " + e.Body | |||
| 216 | + | } | |||
| 217 | + | ||||
| 206 | func parseErrorResponse(resp *http.Response, body []byte) error { | 218 | func parseErrorResponse(resp *http.Response, body []byte) error { | ||
| 219 | + | msg := string(body) | |||
| 207 | var errResp struct { | 220 | var errResp struct { | ||
| 208 | Error string `json:"error"` | 221 | Error string `json:"error"` | ||
| 209 | } | 222 | } | ||
| 210 | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | 223 | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | ||
| 211 | - | return fmt.Errorf("%s: %s", resp.Status, errResp.Error) | 224 | + | msg = errResp.Error |
| 225 | + | } | |||
| 226 | + | return &HTTPError{ | |||
| 227 | + | StatusCode: resp.StatusCode, | |||
| 228 | + | Status: resp.Status, | |||
| 229 | + | Body: msg, | |||
| 212 | } | 230 | } | ||
| 213 | - | return fmt.Errorf("%s: %s", resp.Status, string(body)) | |||
| 214 | } | 231 | } | ||
| 215 | 232 | ||||
| 216 | func isRetryableRequestError(err error) bool { | 233 | func isRetryableRequestError(err error) bool { | ||
M internal/adapters/in/cli/remote/client_test.go +44 -0
| @@ -3,6 +3,8 @@ package remote | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "errors" | |
| 7 | + | "fmt" | |
| 6 | 8 | "io" | |
| 7 | 9 | "net/http" | |
| 8 | 10 | "net/http/httptest" | |
| @@ -304,6 +306,48 @@ func TestExchangeRegistryToken_EmptyToken(t *testing.T) { | |||
| 304 | 306 | assert.Contains(t, err.Error(), "empty") | |
| 305 | 307 | } | |
| 306 | 308 | ||
| 309 | + | func TestHTTPError_ErrorString(t *testing.T) { | |
| 310 | + | err := &HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "insufficient scope"} | |
| 311 | + | assert.Equal(t, "403 Forbidden: insufficient scope", err.Error()) | |
| 312 | + | } | |
| 313 | + | ||
| 314 | + | func TestHTTPError_Unwrap(t *testing.T) { | |
| 315 | + | httpErr := &HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "insufficient scope"} | |
| 316 | + | wrapped := fmt.Errorf("deploy intent: %w", httpErr) | |
| 317 | + | ||
| 318 | + | var target *HTTPError | |
| 319 | + | assert.True(t, errors.As(wrapped, &target)) | |
| 320 | + | assert.Equal(t, 403, target.StatusCode) | |
| 321 | + | } | |
| 322 | + | ||
| 323 | + | func TestParseErrorResponse_ReturnsHTTPError(t *testing.T) { | |
| 324 | + | resp := &http.Response{ | |
| 325 | + | StatusCode: 403, | |
| 326 | + | Status: "403 Forbidden", | |
| 327 | + | Body: io.NopCloser(strings.NewReader("")), | |
| 328 | + | } | |
| 329 | + | err := parseErrorResponse(resp, []byte(`{"error":"insufficient scope"}`)) | |
| 330 | + | ||
| 331 | + | var httpErr *HTTPError | |
| 332 | + | require.True(t, errors.As(err, &httpErr)) | |
| 333 | + | assert.Equal(t, 403, httpErr.StatusCode) | |
| 334 | + | assert.Equal(t, "insufficient scope", httpErr.Body) | |
| 335 | + | } | |
| 336 | + | ||
| 337 | + | func TestParseErrorResponse_NonJSON(t *testing.T) { | |
| 338 | + | resp := &http.Response{ | |
| 339 | + | StatusCode: 500, | |
| 340 | + | Status: "500 Internal Server Error", | |
| 341 | + | Body: io.NopCloser(strings.NewReader("")), | |
| 342 | + | } | |
| 343 | + | err := parseErrorResponse(resp, []byte("plain text error")) | |
| 344 | + | ||
| 345 | + | var httpErr *HTTPError | |
| 346 | + | require.True(t, errors.As(err, &httpErr)) | |
| 347 | + | assert.Equal(t, 500, httpErr.StatusCode) | |
| 348 | + | assert.Equal(t, "plain text error", httpErr.Body) | |
| 349 | + | } | |
| 350 | + | ||
| 307 | 351 | func TestExchangeRegistryToken_AccessTokenFallback(t *testing.T) { | |
| 308 | 352 | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 309 | 353 | w.Header().Set("Content-Type", "application/json") | |
| @@ -3,6 +3,8 @@ package remote | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "errors" | |||
| 7 | + | "fmt" | |||
| 6 | "io" | 8 | "io" | ||
| 7 | "net/http" | 9 | "net/http" | ||
| 8 | "net/http/httptest" | 10 | "net/http/httptest" | ||
| @@ -304,6 +306,48 @@ func TestExchangeRegistryToken_EmptyToken(t *testing.T) { | |||||
| 304 | assert.Contains(t, err.Error(), "empty") | 306 | assert.Contains(t, err.Error(), "empty") | ||
| 305 | } | 307 | } | ||
| 306 | 308 | ||||
| 309 | + | func TestHTTPError_ErrorString(t *testing.T) { | |||
| 310 | + | err := &HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "insufficient scope"} | |||
| 311 | + | assert.Equal(t, "403 Forbidden: insufficient scope", err.Error()) | |||
| 312 | + | } | |||
| 313 | + | ||||
| 314 | + | func TestHTTPError_Unwrap(t *testing.T) { | |||
| 315 | + | httpErr := &HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "insufficient scope"} | |||
| 316 | + | wrapped := fmt.Errorf("deploy intent: %w", httpErr) | |||
| 317 | + | ||||
| 318 | + | var target *HTTPError | |||
| 319 | + | assert.True(t, errors.As(wrapped, &target)) | |||
| 320 | + | assert.Equal(t, 403, target.StatusCode) | |||
| 321 | + | } | |||
| 322 | + | ||||
| 323 | + | func TestParseErrorResponse_ReturnsHTTPError(t *testing.T) { | |||
| 324 | + | resp := &http.Response{ | |||
| 325 | + | StatusCode: 403, | |||
| 326 | + | Status: "403 Forbidden", | |||
| 327 | + | Body: io.NopCloser(strings.NewReader("")), | |||
| 328 | + | } | |||
| 329 | + | err := parseErrorResponse(resp, []byte(`{"error":"insufficient scope"}`)) | |||
| 330 | + | ||||
| 331 | + | var httpErr *HTTPError | |||
| 332 | + | require.True(t, errors.As(err, &httpErr)) | |||
| 333 | + | assert.Equal(t, 403, httpErr.StatusCode) | |||
| 334 | + | assert.Equal(t, "insufficient scope", httpErr.Body) | |||
| 335 | + | } | |||
| 336 | + | ||||
| 337 | + | func TestParseErrorResponse_NonJSON(t *testing.T) { | |||
| 338 | + | resp := &http.Response{ | |||
| 339 | + | StatusCode: 500, | |||
| 340 | + | Status: "500 Internal Server Error", | |||
| 341 | + | Body: io.NopCloser(strings.NewReader("")), | |||
| 342 | + | } | |||
| 343 | + | err := parseErrorResponse(resp, []byte("plain text error")) | |||
| 344 | + | ||||
| 345 | + | var httpErr *HTTPError | |||
| 346 | + | require.True(t, errors.As(err, &httpErr)) | |||
| 347 | + | assert.Equal(t, 500, httpErr.StatusCode) | |||
| 348 | + | assert.Equal(t, "plain text error", httpErr.Body) | |||
| 349 | + | } | |||
| 350 | + | ||||
| 307 | func TestExchangeRegistryToken_AccessTokenFallback(t *testing.T) { | 351 | func TestExchangeRegistryToken_AccessTokenFallback(t *testing.T) { | ||
| 308 | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 352 | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 309 | w.Header().Set("Content-Type", "application/json") | 353 | w.Header().Set("Content-Type", "application/json") | ||