Skip to content

Commit 450b4bb

Browse files
authored
feat(api): configurable upload timeout, default 10m (#160)
* feat(api): add ClientConfig with configurable upload timeout (default 10m) * feat(config): plumb api.uploadTimeout through config * test(config): add GetAPIConfig coverage (defaults + override)
1 parent d09a1d5 commit 450b4bb

6 files changed

Lines changed: 106 additions & 7 deletions

File tree

cmd/ocap_recorder/main.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,14 +289,15 @@ func loadConfig() (err error) {
289289
}
290290

291291
func initAPIClient() {
292-
serverURL := viper.GetString("api.serverUrl")
293-
if serverURL == "" {
292+
apiCfg := config.GetAPIConfig()
293+
if apiCfg.ServerURL == "" {
294294
Logger.Info("API server URL not configured, upload disabled")
295295
return
296296
}
297297

298-
apiKey := viper.GetString("api.apiKey")
299-
apiClient = api.New(serverURL, apiKey)
298+
apiClient = api.NewWithConfig(apiCfg.ServerURL, apiCfg.APIKey, api.ClientConfig{
299+
UploadTimeout: apiCfg.UploadTimeout,
300+
})
300301

301302
if err := apiClient.Healthcheck(); err != nil {
302303
Logger.Info("OCAP Frontend is offline", "error", err)

internal/api/client.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,42 @@ import (
1818
// PathPrefix is the API path prefix appended to the base server URL.
1919
const PathPrefix = "/api"
2020

21+
// DefaultUploadTimeout is the default upload timeout when the caller
22+
// does not provide one. It covers the full request (including body
23+
// streaming), so it must be generous enough for tens-of-MB uploads
24+
// across a slow reverse proxy.
25+
const DefaultUploadTimeout = 10 * time.Minute
26+
27+
// ClientConfig configures the API client. All fields are optional —
28+
// zero values resolve to sensible defaults.
29+
type ClientConfig struct {
30+
// UploadTimeout is the maximum duration for the total upload request
31+
// (including streaming the body). Defaults to DefaultUploadTimeout.
32+
UploadTimeout time.Duration
33+
}
34+
2135
// Client handles communication with the OCAP web frontend.
2236
type Client struct {
2337
baseURL string
2438
apiKey string
2539
httpClient *http.Client
2640
}
2741

28-
// New creates a new API client.
42+
// New creates a new API client with default configuration.
2943
func New(baseURL, apiKey string) *Client {
44+
return NewWithConfig(baseURL, apiKey, ClientConfig{})
45+
}
46+
47+
// NewWithConfig creates a new API client with custom configuration.
48+
func NewWithConfig(baseURL, apiKey string, cfg ClientConfig) *Client {
49+
timeout := cfg.UploadTimeout
50+
if timeout <= 0 {
51+
timeout = DefaultUploadTimeout
52+
}
3053
return &Client{
3154
baseURL: strings.TrimRight(baseURL, "/") + PathPrefix,
3255
apiKey: apiKey,
33-
httpClient: &http.Client{Timeout: 30 * time.Second},
56+
httpClient: &http.Client{Timeout: timeout},
3457
}
3558
}
3659

internal/api/client_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http/httptest"
77
"os"
88
"testing"
9+
"time"
910

1011
"github.com/OCAP2/extension/v5/pkg/core"
1112
"github.com/stretchr/testify/assert"
@@ -193,6 +194,27 @@ func TestUpload_InvalidURL(t *testing.T) {
193194
assert.Contains(t, err.Error(), "failed to create request")
194195
}
195196

197+
func TestNew_DefaultTimeouts(t *testing.T) {
198+
c := New("http://localhost:5000", "secret")
199+
require.NotNil(t, c)
200+
assert.NotNil(t, c.httpClient)
201+
// Backwards-compat default preserved: 10 minutes.
202+
assert.Equal(t, 10*time.Minute, c.httpClient.Timeout)
203+
}
204+
205+
func TestNewWithConfig_CustomTimeout(t *testing.T) {
206+
c := NewWithConfig("http://localhost:5000", "secret", ClientConfig{
207+
UploadTimeout: 2 * time.Minute,
208+
})
209+
require.NotNil(t, c)
210+
assert.Equal(t, 2*time.Minute, c.httpClient.Timeout)
211+
}
212+
213+
func TestNewWithConfig_ZeroTimeoutUsesDefault(t *testing.T) {
214+
c := NewWithConfig("http://localhost:5000", "secret", ClientConfig{UploadTimeout: 0})
215+
assert.Equal(t, 10*time.Minute, c.httpClient.Timeout)
216+
}
217+
196218
func writeTestFile(path string, content []byte) error {
197219
return os.WriteFile(path, content, 0644)
198220
}

internal/config/config.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ func Load(configDir string) error {
2828

2929
viper.SetDefault("api.serverUrl", "http://localhost:5000")
3030
viper.SetDefault("api.apiKey", "")
31+
// 10 minute default — generous enough for multi-hundred-MB uploads across
32+
// a reverse proxy without being so long that a dead backend hangs the save
33+
// worker forever.
34+
viper.SetDefault("api.uploadTimeout", "10m")
3135

3236
viper.SetDefault("db.host", "localhost")
3337
viper.SetDefault("db.port", "5432")
@@ -113,3 +117,17 @@ func GetOTelConfig() OTelConfig {
113117
_ = viper.UnmarshalKey("otel", &cfg)
114118
return cfg
115119
}
120+
121+
// APIConfig holds HTTP client configuration for the OCAP web API.
122+
type APIConfig struct {
123+
ServerURL string `json:"serverUrl" mapstructure:"serverUrl"`
124+
APIKey string `json:"apiKey" mapstructure:"apiKey"`
125+
UploadTimeout time.Duration `json:"uploadTimeout" mapstructure:"uploadTimeout"`
126+
}
127+
128+
// GetAPIConfig returns the HTTP API client configuration.
129+
func GetAPIConfig() APIConfig {
130+
var cfg APIConfig
131+
_ = viper.UnmarshalKey("api", &cfg)
132+
return cfg
133+
}

internal/config/config_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func TestLoad_DefaultValues(t *testing.T) {
4545
assert.Equal(t, "./ocaplogs", viper.GetString("logsDir"))
4646
assert.Equal(t, "http://localhost:5000", viper.GetString("api.serverUrl"))
4747
assert.Equal(t, "", viper.GetString("api.apiKey"))
48+
assert.Equal(t, "10m", viper.GetString("api.uploadTimeout"))
4849
assert.Equal(t, "localhost", viper.GetString("db.host"))
4950
assert.Equal(t, "5432", viper.GetString("db.port"))
5051
assert.Equal(t, "postgres", viper.GetString("db.username"))
@@ -127,6 +128,39 @@ func TestGetStorageConfig_Override(t *testing.T) {
127128
assert.Equal(t, 10*time.Minute, sc.SQLite.DumpInterval)
128129
}
129130

131+
func TestGetAPIConfig_Defaults(t *testing.T) {
132+
t.Cleanup(viper.Reset)
133+
134+
dir := t.TempDir()
135+
require.NoError(t, os.WriteFile(filepath.Join(dir, "ocap_recorder.cfg.json"), []byte(`{}`), 0644))
136+
require.NoError(t, Load(dir))
137+
138+
cfg := GetAPIConfig()
139+
assert.Equal(t, "http://localhost:5000", cfg.ServerURL)
140+
assert.Equal(t, "", cfg.APIKey)
141+
assert.Equal(t, 10*time.Minute, cfg.UploadTimeout)
142+
}
143+
144+
func TestGetAPIConfig_Override(t *testing.T) {
145+
t.Cleanup(viper.Reset)
146+
147+
dir := t.TempDir()
148+
cfgJSON := `{
149+
"api": {
150+
"serverUrl": "http://example.com:8080",
151+
"apiKey": "mykey",
152+
"uploadTimeout": "5m"
153+
}
154+
}`
155+
require.NoError(t, os.WriteFile(filepath.Join(dir, "ocap_recorder.cfg.json"), []byte(cfgJSON), 0644))
156+
require.NoError(t, Load(dir))
157+
158+
cfg := GetAPIConfig()
159+
assert.Equal(t, "http://example.com:8080", cfg.ServerURL)
160+
assert.Equal(t, "mykey", cfg.APIKey)
161+
assert.Equal(t, 5*time.Minute, cfg.UploadTimeout)
162+
}
163+
130164
func TestGetOTelConfig_Defaults(t *testing.T) {
131165
t.Cleanup(viper.Reset)
132166

ocap_recorder.cfg.json.example

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"defaultTag": "TvT",
55
"api": {
66
"serverUrl": "http://127.0.0.1:5000",
7-
"apiKey": "same-secret"
7+
"apiKey": "same-secret",
8+
"uploadTimeout": "10m"
89
},
910
"db": {
1011
"host": "127.0.0.1",

0 commit comments

Comments
 (0)