Skip to content

Commit e42cb61

Browse files
committed
Implement phase 3 SDK enhancements
1 parent e1f45e8 commit e42cb61

File tree

14 files changed

+933
-85
lines changed

14 files changed

+933
-85
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ BREAKING CHANGES:
1111
* Remove `file.OrderBySizeAsc` and `file.OrderBySizeDesc` constants (not supported in v0.7)
1212
* Remove `APIv05` and `APIv06` constants
1313
* Minimum Go version is now 1.25
14+
* Throttled requests no longer retry by default — automatic retries are now opt-in via `ucare.Config.Retry`
1415

1516
FEATURES:
1617

@@ -19,12 +20,18 @@ FEATURES:
1920
* Add `metadata` package with file metadata CRUD operations
2021
* Add `group.Delete()` for deleting group metadata without deleting files
2122
* Add webhook event constants for `file.stored`, `file.deleted`, `file.info_updated`, and deprecated `file.infected`
23+
* Add `ucare.Config.Retry` and `RetryConfig` for configurable throttling retries
24+
* Add `upload.Service.Upload()` for automatic direct-vs-multipart upload selection
25+
* Export structured API error types: `APIError`, `AuthError`, `ThrottleError`, `ValidationError`, and `ForbiddenError`
2226

2327
IMPROVEMENTS:
2428

2529
* Add `UserAgent` field to `ucare.Config` for custom agent identification
2630
* Replace `http.NewRequest` + `WithContext` with `http.NewRequestWithContext`
2731
* Throttle retry loops now respect context cancellation
32+
* REST throttling retries now honor `Retry-After` with configurable fail-fast limits and fallback exponential backoff
33+
* Upload throttling retries now use configurable exponential backoff capping
34+
* Error values now expose HTTP status details for caller inspection
2835
* Replace `ioutil` usage with `io` equivalents
2936
* Replace `go-env` dependency with `os.Getenv`
3037
* Update `stretchr/testify` to v1.10.0

internal/config/config.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ const (
1919

2020
AcceptHeaderFormat = "application/vnd.uploadcare-%s+json"
2121

22-
MaxThrottleRetries = 3
23-
2422
UCTimeLayout = "2006-01-02T15:04:05"
2523
)
2624

ucare/client.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ type Config struct {
4040
// UserAgent is appended to the default User-Agent string.
4141
// Use this to identify your application (e.g. "my-app/1.0.0").
4242
UserAgent string
43+
// Retry controls automatic retry of throttled (HTTP 429) requests.
44+
// When nil (the default), throttled requests fail immediately.
45+
Retry *RetryConfig
4346
}
4447

4548
// ReqEncoder exists to encode data into the prepared request.

ucare/error.go

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"fmt"
66
)
77

8-
// API response errors
8+
// Sentinel errors for specific conditions
99
var (
1010
ErrInvalidAuthCreds = errors.New("Incorrect authentication credentials")
1111
ErrAuthForbidden = errors.New("Simple authentication over HTTP is " +
@@ -18,43 +18,50 @@ var (
1818
"files smaller than 100MB")
1919
)
2020

21-
type respErr struct {
22-
Details string `json:"detail"`
21+
// APIError represents a generic API error response.
22+
// Callers can use errors.As to check for this type as a catch-all
23+
// for any non-specific API error.
24+
type APIError struct {
25+
StatusCode int `json:"-"`
26+
Detail string `json:"detail"`
2327
}
2428

25-
// Error implements error interface
26-
func (e respErr) Error() string {
27-
return e.Details
29+
func (e APIError) Error() string {
30+
return fmt.Sprintf("uploadcare: HTTP %d: %s", e.StatusCode, e.Detail)
2831
}
2932

30-
type authErr struct{ respErr }
33+
// AuthError represents an authentication failure (HTTP 401).
34+
type AuthError struct{ APIError }
3135

32-
type throttleErr struct {
36+
func (e AuthError) Error() string {
37+
return fmt.Sprintf("uploadcare: authentication failed: %s", e.Detail)
38+
}
39+
40+
// ThrottleError represents a throttled request (HTTP 429).
41+
type ThrottleError struct {
3342
RetryAfter int
3443
}
3544

36-
func (e throttleErr) Error() string {
45+
func (e ThrottleError) Error() string {
3746
if e.RetryAfter == 0 {
38-
return "Request was throttled."
47+
return "uploadcare: request throttled"
3948
}
4049
return fmt.Sprintf(
41-
"Request was throttled. Expected available in %d second",
50+
"uploadcare: request throttled, retry after %d seconds",
4251
e.RetryAfter,
4352
)
4453
}
4554

46-
type reqValidationErr struct{ respErr }
55+
// ValidationError represents a request validation failure (HTTP 400).
56+
type ValidationError struct{ APIError }
4757

48-
func (e reqValidationErr) Error() string {
49-
return fmt.Sprintf("Request parameters validation error: %s", e.Details)
58+
func (e ValidationError) Error() string {
59+
return fmt.Sprintf("uploadcare: validation error: %s", e.Detail)
5060
}
5161

52-
type reqForbiddenErr struct{ respErr }
53-
54-
type unexpectedStatusErr struct {
55-
StatusCode int
56-
}
62+
// ForbiddenError represents a forbidden request (HTTP 403).
63+
type ForbiddenError struct{ APIError }
5764

58-
func (e unexpectedStatusErr) Error() string {
59-
return fmt.Sprintf("unexpected HTTP status: %d", e.StatusCode)
65+
func (e ForbiddenError) Error() string {
66+
return fmt.Sprintf("uploadcare: forbidden: %s", e.Detail)
6067
}

ucare/error_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package ucare
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
assert "github.com/stretchr/testify/require"
8+
)
9+
10+
func TestAPIError_Error(t *testing.T) {
11+
t.Parallel()
12+
err := APIError{StatusCode: 404, Detail: "not found"}
13+
assert.Equal(t, "uploadcare: HTTP 404: not found", err.Error())
14+
}
15+
16+
func TestAuthError_Error(t *testing.T) {
17+
t.Parallel()
18+
err := AuthError{APIError{StatusCode: 401, Detail: "invalid token"}}
19+
assert.Equal(t, "uploadcare: authentication failed: invalid token", err.Error())
20+
}
21+
22+
func TestThrottleError_Error(t *testing.T) {
23+
t.Parallel()
24+
err := ThrottleError{RetryAfter: 5}
25+
assert.Equal(t, "uploadcare: request throttled, retry after 5 seconds", err.Error())
26+
}
27+
28+
func TestThrottleError_Error_ZeroRetryAfter(t *testing.T) {
29+
t.Parallel()
30+
err := ThrottleError{}
31+
assert.Equal(t, "uploadcare: request throttled", err.Error())
32+
}
33+
34+
func TestValidationError_Error(t *testing.T) {
35+
t.Parallel()
36+
err := ValidationError{APIError{StatusCode: 400, Detail: "bad field"}}
37+
assert.Equal(t, "uploadcare: validation error: bad field", err.Error())
38+
}
39+
40+
func TestForbiddenError_Error(t *testing.T) {
41+
t.Parallel()
42+
err := ForbiddenError{APIError{StatusCode: 403, Detail: "denied"}}
43+
assert.Equal(t, "uploadcare: forbidden: denied", err.Error())
44+
}
45+
46+
func TestErrorsAs_APIError(t *testing.T) {
47+
t.Parallel()
48+
var target APIError
49+
err := error(APIError{StatusCode: 409, Detail: "conflict"})
50+
assert.True(t, errors.As(err, &target))
51+
assert.Equal(t, 409, target.StatusCode)
52+
}
53+
54+
func TestErrorsAs_AuthError(t *testing.T) {
55+
t.Parallel()
56+
var target AuthError
57+
err := error(AuthError{APIError{StatusCode: 401, Detail: "bad creds"}})
58+
assert.True(t, errors.As(err, &target))
59+
assert.Equal(t, 401, target.StatusCode)
60+
}
61+
62+
func TestErrorsAs_ThrottleError(t *testing.T) {
63+
t.Parallel()
64+
var target ThrottleError
65+
err := error(ThrottleError{RetryAfter: 10})
66+
assert.True(t, errors.As(err, &target))
67+
assert.Equal(t, 10, target.RetryAfter)
68+
}
69+
70+
func TestErrorsAs_AuthError_DoesNotMatchAPIError(t *testing.T) {
71+
t.Parallel()
72+
var target APIError
73+
err := error(AuthError{APIError{StatusCode: 401, Detail: "bad creds"}})
74+
// AuthError embeds APIError as a value, not a pointer.
75+
// errors.As does not unwrap embedded value types, so this does NOT match.
76+
// Callers should check specific types first, then APIError as fallback.
77+
assert.False(t, errors.As(err, &target))
78+
}
79+
80+
func TestErrorsAs_ValidationError(t *testing.T) {
81+
t.Parallel()
82+
var target ValidationError
83+
err := error(ValidationError{APIError{StatusCode: 400, Detail: "bad input"}})
84+
assert.True(t, errors.As(err, &target))
85+
assert.Equal(t, 400, target.StatusCode)
86+
}
87+
88+
func TestErrorsAs_ForbiddenError(t *testing.T) {
89+
t.Parallel()
90+
var target ForbiddenError
91+
err := error(ForbiddenError{APIError{StatusCode: 403, Detail: "no"}})
92+
assert.True(t, errors.As(err, &target))
93+
assert.Equal(t, 403, target.StatusCode)
94+
}

ucare/restapi.go

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ type restAPIClient struct {
2222
acceptHeader string
2323
setAuthHeader restAPIAuthFunc
2424

25-
conn *http.Client
25+
conn *http.Client
26+
retry *RetryConfig
2627
}
2728

2829
func newRESTAPIClient(creds APICreds, conf *Config) Client {
@@ -32,7 +33,8 @@ func newRESTAPIClient(creds APICreds, conf *Config) Client {
3233

3334
setAuthHeader: simpleRESTAPIAuth,
3435

35-
conn: conf.HTTPClient,
36+
conn: conf.HTTPClient,
37+
retry: conf.Retry,
3638
}
3739

3840
if conf.SignBasedAuthentication {
@@ -131,43 +133,59 @@ func (c *restAPIClient) handleResponse(
131133

132134
switch resp.StatusCode {
133135
case 400, 404:
134-
var err respErr
135-
if e := json.NewDecoder(resp.Body).Decode(&err); e != nil {
136+
var apiErr APIError
137+
if e := json.NewDecoder(resp.Body).Decode(&apiErr); e != nil {
136138
return false, e
137139
}
138-
return false, err
140+
apiErr.StatusCode = resp.StatusCode
141+
return false, apiErr
139142
case 401:
140-
var err authErr
141-
if e := json.NewDecoder(resp.Body).Decode(&err); e != nil {
143+
var authErr AuthError
144+
if e := json.NewDecoder(resp.Body).Decode(&authErr); e != nil {
142145
return false, e
143146
}
144-
return false, err
147+
authErr.StatusCode = 401
148+
return false, authErr
149+
case 403:
150+
var forbiddenErr ForbiddenError
151+
if e := json.NewDecoder(resp.Body).Decode(&forbiddenErr); e != nil {
152+
return false, e
153+
}
154+
forbiddenErr.StatusCode = 403
155+
return false, forbiddenErr
145156
case 406:
146157
return false, ErrInvalidVersion
147158
case 429:
148159
retryAfter, err := strconv.Atoi(
149160
resp.Header.Get("Retry-After"),
150161
)
151162
if err != nil {
152-
return false, fmt.Errorf("invalid Retry-After: %w", err)
163+
retryAfter = 0
153164
}
154-
155-
if tries > config.MaxThrottleRetries {
156-
return false, throttleErr{retryAfter}
165+
if c.retry == nil || tries > c.retry.MaxRetries {
166+
return false, ThrottleError{RetryAfter: retryAfter}
167+
}
168+
if c.retry.MaxWaitSeconds > 0 &&
169+
retryAfter > c.retry.MaxWaitSeconds {
170+
return false, ThrottleError{RetryAfter: retryAfter}
171+
}
172+
wait := retryAfter
173+
if wait <= 0 {
174+
wait = expBackoff(tries)
157175
}
158-
159176
select {
160177
case <-req.Context().Done():
161178
return false, req.Context().Err()
162-
case <-time.After(time.Duration(retryAfter) * time.Second):
179+
case <-time.After(time.Duration(wait) * time.Second):
163180
}
164181
return true, nil
165182
default:
166183
if resp.StatusCode >= 400 {
167-
var apiErr respErr
168-
if e := json.NewDecoder(resp.Body).Decode(&apiErr); e != nil || apiErr.Details == "" {
169-
return false, unexpectedStatusErr{StatusCode: resp.StatusCode}
184+
var apiErr APIError
185+
if e := json.NewDecoder(resp.Body).Decode(&apiErr); e != nil || apiErr.Detail == "" {
186+
apiErr.Detail = http.StatusText(resp.StatusCode)
170187
}
188+
apiErr.StatusCode = resp.StatusCode
171189
return false, apiErr
172190
}
173191
}

0 commit comments

Comments
 (0)