Skip to content

Commit 754fe3e

Browse files
committed
Update config loader to return clearer err message on invalid duration format
1 parent 8cfc25e commit 754fe3e

File tree

7 files changed

+74
-34
lines changed

7 files changed

+74
-34
lines changed

internal/agent/api/action_api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func (s *Server) handleYcrashForward(writer http.ResponseWriter, request *http.R
176176
fr.Header.Del("ycrash-forward")
177177
fr.Close = true
178178
client := http.Client{
179-
Timeout: config.GlobalConfig.HttpClientTimeout,
179+
Timeout: config.GlobalConfig.HttpClientTimeout.Duration(),
180180
}
181181
r, err := client.Do(fr)
182182
if err != nil {

internal/agent/m3/m3.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func NewM3App() *M3App {
4343
func (m3 *M3App) RunLoop() {
4444
for {
4545
m3.RunSingle()
46-
time.Sleep(config.GlobalConfig.M3Frequency)
46+
time.Sleep(config.GlobalConfig.M3Frequency.Duration())
4747
}
4848
}
4949

internal/agent/ondemand/ondemand.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ Ignored errors: %v
356356
Pid: pid,
357357
TdPath: tdPath,
358358
JavaHome: config.GlobalConfig.JavaHomePath,
359-
TdCaptureDuration: config.GlobalConfig.TDCaptureDuration,
359+
TdCaptureDuration: config.GlobalConfig.TDCaptureDuration.Duration(),
360360
}
361361
threadDump = goCapture(endpoint, capture.WrapRun(capThreadDump))
362362

@@ -997,7 +997,7 @@ func RequestFin(endpoint string) (resp []byte, err error) {
997997
}
998998
httpClient := &http.Client{
999999
Transport: transport,
1000-
Timeout: config.GlobalConfig.HttpClientTimeout,
1000+
Timeout: config.GlobalConfig.HttpClientTimeout.Duration(),
10011001
}
10021002
req, err := http.NewRequest("POST", endpoint, nil)
10031003
if err != nil {

internal/capture/executils/shell.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func CommandCombinedOutputToWriter(writer io.Writer, cmd Command, hookers ...Hoo
197197
}()
198198

199199
// execution timer
200-
timerDuration := config.GlobalConfig.CmdTimeout
200+
timerDuration := config.GlobalConfig.CmdTimeout.Duration()
201201
if timerDuration == 0 {
202202
timerDuration = 60 * time.Second // fallback to default if not configured, or accidentally set to 0
203203
}

internal/capture/healthcheck.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func newHTTPClient() *http.Client {
150150
DisableKeepAlives: true,
151151
ForceAttemptHTTP2: true,
152152
},
153-
Timeout: config.GlobalConfig.HttpClientTimeout,
153+
Timeout: config.GlobalConfig.HttpClientTimeout.Duration(),
154154
}
155155
}
156156

internal/capture/post.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func PostCustomDataWithTimeout(endpoint, params string, file *os.File, timeout t
8686
}
8787

8888
func PostCustomDataWithPositionFunc(endpoint, params string, file *os.File, position func(file *os.File) error) (msg string, ok bool) {
89-
return PostCustomDataWithPositionFuncWithTimeout(endpoint, params, file, position, config.GlobalConfig.HttpClientTimeout)
89+
return PostCustomDataWithPositionFuncWithTimeout(endpoint, params, file, position, config.GlobalConfig.HttpClientTimeout.Duration())
9090
}
9191

9292
func PostCustomDataWithPositionFuncWithTimeout(endpoint, params string, file *os.File, position func(file *os.File) error, timeout time.Duration) (msg string, ok bool) {
@@ -183,7 +183,7 @@ func GetData(endpoint string) (msg string, ok bool) {
183183
}
184184
httpClient := &http.Client{
185185
Transport: transport,
186-
Timeout: config.GlobalConfig.HttpClientTimeout,
186+
Timeout: config.GlobalConfig.HttpClientTimeout.Duration(),
187187
}
188188
req, err := http.NewRequest("GET", endpoint, nil)
189189
if err != nil {

internal/config/config.go

Lines changed: 66 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,17 @@ type Config struct {
2020
}
2121

2222
type Options struct {
23-
Pid string `yaml:"p" usage:"The process ID or unique token of the target, for example: 3121, or 'buggyApp'"`
24-
ApiKey string `yaml:"k" usage:"The API Key that will be used to make API requests, for example: tier1app@12312-12233-1442134-112"`
25-
Server string `yaml:"s" usage:"The server url that will be used to upload data, for example: https://ycrash.companyname.com"`
26-
AppName string `yaml:"a" usage:"The APP Name of the target"`
27-
HeapDump bool `yaml:"hd" usage:"Capture heap dump, default is false"`
28-
HeapDumpPath string `yaml:"hdPath" usage:"The heap dump file to be uploaded while it exists"`
29-
ThreadDumpPath string `yaml:"tdPath" usage:"The thread dump file to be uploaded while it exists"`
30-
TDCaptureDuration time.Duration `yaml:"tdCaptureDuration" usage:"Total duration to capture thread dumps (e.g., 10m, 30s)"`
31-
GCPath string `yaml:"gcPath" usage:"The gc log file to be uploaded while it exists"`
32-
JavaHomePath string `yaml:"j" usage:"The java home path to be used. Default will try to use os env 'JAVA_HOME' if 'JAVA_HOME' is not empty, for example: /usr/lib/jvm/java-8-openjdk-amd64"`
33-
DeferDelete bool `yaml:"d" usage:"Delete logs folder created during analyse"`
23+
Pid string `yaml:"p" usage:"The process ID or unique token of the target, for example: 3121, or 'buggyApp'"`
24+
ApiKey string `yaml:"k" usage:"The API Key that will be used to make API requests, for example: tier1app@12312-12233-1442134-112"`
25+
Server string `yaml:"s" usage:"The server url that will be used to upload data, for example: https://ycrash.companyname.com"`
26+
AppName string `yaml:"a" usage:"The APP Name of the target"`
27+
HeapDump bool `yaml:"hd" usage:"Capture heap dump, default is false"`
28+
HeapDumpPath string `yaml:"hdPath" usage:"The heap dump file to be uploaded while it exists"`
29+
ThreadDumpPath string `yaml:"tdPath" usage:"The thread dump file to be uploaded while it exists"`
30+
TDCaptureDuration Duration `yaml:"tdCaptureDuration" usage:"Total duration to capture thread dumps (e.g., 10m, 30s)"`
31+
GCPath string `yaml:"gcPath" usage:"The gc log file to be uploaded while it exists"`
32+
JavaHomePath string `yaml:"j" usage:"The java home path to be used. Default will try to use os env 'JAVA_HOME' if 'JAVA_HOME' is not empty, for example: /usr/lib/jvm/java-8-openjdk-amd64"`
33+
DeferDelete bool `yaml:"d" usage:"Delete logs folder created during analyse"`
3434

3535
ShowVersion bool `arg:"version" yaml:"-" usage:"Show the version of this program"`
3636
ConfigPath string `arg:"c" yaml:"-" usage:"The config file path to load"`
@@ -41,7 +41,7 @@ type Options struct {
4141
CACertPath string `yaml:"caCertPath" usage:"The CA Cert Path"`
4242

4343
M3 bool `arg:"m3" usage:"Run in m3 mode, default is false"`
44-
M3Frequency time.Duration `yaml:"m3Frequency" usage:"Frequency of m3 mode, default is 3 minutes"`
44+
M3Frequency Duration `yaml:"m3Frequency" usage:"Frequency of m3 mode, default is 3 minutes"`
4545
ProcessTokens ProcessTokens `yaml:"processTokens" usage:"Process tokens of m3 mode"`
4646
ExcludeProcessTokens ProcessTokens `yaml:"excludeTokens" usage:"Process exclude tokens of m3 mode"`
4747

@@ -94,22 +94,24 @@ type Options struct {
9494
EdScript string `yaml:"edScript" usage:"Extended Data: Path to a custom script that collects extended diagnostics data"`
9595
EdDataFolder string `yaml:"edDataFolder" usage:"Extended Data: Directory path where artifacts generated by (-edScript) will be stored and collected."`
9696

97-
CmdTimeout time.Duration `yaml:"cmdTimeout" usage:"Command execution timeout duration (e.g., 1m, 30s). Default is 60 seconds."`
98-
HttpClientTimeout time.Duration `yaml:"httpClientTimeout" usage:"HTTP client timeout for API requests (e.g., 1m, 30s). Default is 60 seconds."`
97+
CmdTimeout Duration `yaml:"cmdTimeout" usage:"Command execution timeout duration (e.g., 1m, 30s). Default is 60 seconds."`
98+
HttpClientTimeout Duration `yaml:"httpClientTimeout" usage:"HTTP client timeout for API requests (e.g., 1m, 30s). Default is 60 seconds."`
9999
}
100100

101-
type HealthChecks map[string]HealthCheck
101+
type Command struct {
102+
UrlParams UrlParams `yaml:"urlParams" usage:"[DEPRECATED] This option is no longer in use."`
103+
Cmd Cmd `yaml:"cmd" usage:"[DEPRECATED] This option is no longer in use."`
104+
}
105+
106+
// Healthcheck
102107
type HealthCheck struct {
103108
Endpoint string `yaml:"endpoint"`
104109
HTTPBody string `yaml:"httpBody"`
105110
TimeoutSecs int `yaml:"timeoutSecs"`
106111
}
112+
type HealthChecks map[string]HealthCheck
107113

108-
type Command struct {
109-
UrlParams UrlParams `yaml:"urlParams" usage:"[DEPRECATED] This option is no longer in use."`
110-
Cmd Cmd `yaml:"cmd" usage:"[DEPRECATED] This option is no longer in use."`
111-
}
112-
114+
// UrlParams
113115
type UrlParams string
114116
type UrlParamsSlice []UrlParams
115117

@@ -122,6 +124,7 @@ func (u *UrlParamsSlice) Set(p string) error {
122124
return nil
123125
}
124126

127+
// Cmd
125128
type Cmd string
126129
type CmdSlice []Cmd
127130

@@ -139,6 +142,7 @@ type CommandsFlagSetPair struct {
139142
CmdSlice
140143
}
141144

145+
// ProcessToken
142146
type ProcessToken string
143147
type ProcessTokens []ProcessToken
144148

@@ -151,6 +155,7 @@ func (p *ProcessTokens) Set(s string) error {
151155
return nil
152156
}
153157

158+
// AppLog
154159
type AppLog string
155160
type AppLogs []AppLog
156161

@@ -163,11 +168,39 @@ func (p *AppLogs) Set(s string) error {
163168
return nil
164169
}
165170

171+
// Duration wraps time.Duration to provide better YAML unmarshaling with user-friendly error messages
172+
type Duration time.Duration
173+
174+
func (d *Duration) UnmarshalYAML(unmarshal func(interface{}) error) error {
175+
var s string
176+
if err := unmarshal(&s); err != nil {
177+
return err
178+
}
179+
180+
duration, err := time.ParseDuration(s)
181+
if err != nil {
182+
return fmt.Errorf("invalid duration format '%s': %w (expected format like '1m', '30s', '1h30m')", s, err)
183+
}
184+
185+
*d = Duration(duration)
186+
return nil
187+
}
188+
189+
// Duration converts back to time.Duration when needed
190+
func (d Duration) Duration() time.Duration {
191+
return time.Duration(d)
192+
}
193+
194+
// String returns the string representation of the duration
195+
func (d Duration) String() string {
196+
return time.Duration(d).String()
197+
}
198+
166199
func defaultConfig() Config {
167200
return Config{
168201
Options: Options{
169202
VerifySSL: true,
170-
M3Frequency: 3 * time.Minute,
203+
M3Frequency: Duration(3 * time.Minute),
171204
Address: "localhost",
172205
Port: -1,
173206
LogFileMaxCount: 7,
@@ -176,9 +209,9 @@ func defaultConfig() Config {
176209
PingHost: "google.com",
177210
DeferDelete: true,
178211
AppLogLineCount: 10000,
179-
TDCaptureDuration: 0 * time.Second, // Setting here 0 seconds as default since handling it in jstack.go
180-
CmdTimeout: 60 * time.Second,
181-
HttpClientTimeout: 60 * time.Second,
212+
TDCaptureDuration: Duration(0 * time.Second), // Setting here 0 seconds as default since handling it in jstack.go
213+
CmdTimeout: Duration(60 * time.Second),
214+
HttpClientTimeout: Duration(60 * time.Second),
182215
},
183216
}
184217
}
@@ -248,13 +281,20 @@ func copyFlagsValue(dst interface{}, src map[int]interface{}) (err error) {
248281
var x reflect.Value
249282
fieldValue := value.Field(i)
250283
name := typ.Field(i).Name
284+
fieldType := typ.Field(i)
285+
251286
if name == "VerifySSL" {
252287
bs := *(s).(*string)
253288
b, err := strconv.ParseBool(strings.ToLower(bs))
254289
if err != nil {
255290
return fmt.Errorf("-verifySSL should be true or false, not %s", bs)
256291
}
257292
x = reflect.ValueOf(b)
293+
} else if fieldType.Type == reflect.TypeOf(Duration(0)) {
294+
// Handle Duration fields - convert from *time.Duration to Duration
295+
timeDurationPtr := s.(*time.Duration)
296+
duration := Duration(*timeDurationPtr)
297+
x = reflect.ValueOf(duration)
258298
} else {
259299
x = reflect.ValueOf(s).Elem()
260300
}
@@ -299,8 +339,8 @@ func registerFlags(flagSetName string) (*flag.FlagSet, map[int]interface{}) {
299339
flagSet.Var(&appLogs, name, usage)
300340
result[i] = &appLogs
301341
continue
302-
case time.Duration:
303-
result[i] = flagSet.Duration(name, v, usage)
342+
case Duration:
343+
result[i] = flagSet.Duration(name, v.Duration(), usage)
304344
continue
305345
case HealthChecks:
306346
// Ignore this due to nested structure, we don't support this via CLI for now.

0 commit comments

Comments
 (0)