From b067266e6b838e201069b9a1c928fbf7f8db5537 Mon Sep 17 00:00:00 2001 From: anant Date: Sun, 15 Nov 2026 00:47:52 +0530 Subject: [PATCH] shell: fix s3.circuitBreaker help examples and error messages (#12657) * shell: fix s3.circuitBreaker help examples and error messages The help examples used -type count, but -type accepts Count and MB (as its flag description says), so both examples failed with "type not valid, only 'count' and 'bytes' are allowed", an error that names neither accepted value. Two other errors pointed at flags that do not exist: -max and -counts, where the flag is -values. Use -type Count in the examples, name Count and MB in the type error, and -values in the value errors. TestCircuitBreakerHelpExamplesRun runs every help example and fails on master for the two -type count lines. Signed-off-by: anant <170962632+antcybersec@users.noreply.github.com> * shell: name the bad action and the -actions flag in s3.circuitBreaker The unknown-action error pointed at a nonexistent -action flag and echoed the whole -actions list instead of the action that was rejected. Co-Authored-By: Claude Opus 5.5 --------- Signed-off-by: anant <170962632+antcybersec@users.noreply.github.com> Co-authored-by: anant <170962632+antcybersec@users.noreply.github.com> Co-authored-by: Chris Lu Co-authored-by: Claude Opus 5.5 --- weed/shell/command_s3_circuitbreaker.go | 16 +++--- .../command_s3_circuitbreaker_help_test.go | 53 +++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) create mode 100644 weed/shell/command_s3_circuitbreaker_help_test.go diff --git a/weed/shell/command_s3_circuitbreaker.go b/weed/shell/command_s3_circuitbreaker.go index b73955ab96..2827b9dd44 100644 --- a/weed/shell/command_s3_circuitbreaker.go +++ b/weed/shell/command_s3_circuitbreaker.go @@ -34,13 +34,13 @@ func (c *commandS3CircuitBreaker) Help() string { # examples # add circuit breaker config for global - s3.circuitBreaker -global -type count -actions Read,Write -values 500,200 -apply + s3.circuitBreaker -global -type Count -actions Read,Write -values 500,200 -apply # disable global config s3.circuitBreaker -global -disable -apply # add circuit breaker config for buckets x,y,z - s3.circuitBreaker -buckets x,y,z -type count -actions Read,Write -values 200,100 -apply + s3.circuitBreaker -buckets x,y,z -type Count -actions Read,Write -values 200,100 -apply # disable circuit breaker config of x s3.circuitBreaker -buckets x -disable -apply @@ -207,7 +207,7 @@ func loadConfig(commandEnv *CommandEnv, dir string, file string, buf *bytes.Buff func insertOrUpdateValues(cbOptions *s3_pb.S3CircuitBreakerOptions, cmdActions []string, cmdValues []int64, limitType *string) error { if len(*limitType) == 0 { - return fmt.Errorf("type not valid, only 'count' and 'bytes' are allowed") + return fmt.Errorf("type not valid, only 'Count' and 'MB' are allowed") } if cbOptions.Actions == nil { @@ -289,7 +289,7 @@ func (c *commandS3CircuitBreaker) initActionsAndValues(buckets, actions, limitTy } } if !found { - return nil, nil, nil, fmt.Errorf("value(%s) of flag[-action] not valid, allowed actions: %v", *actions, s3_constants.AllowedActions) + return nil, nil, nil, fmt.Errorf("value(%s) of flag[-actions] not valid, allowed actions: %v", action, s3_constants.AllowedActions) } } } @@ -307,7 +307,7 @@ func (c *commandS3CircuitBreaker) initActionsAndValues(buckets, actions, limitTy elements := strings.Split(*values, ",") if len(cmdActions) != len(elements) { if len(elements) != 1 || len(elements) == 0 { - return nil, nil, nil, fmt.Errorf("count of flag[-actions] and flag[-counts] not equal") + return nil, nil, nil, fmt.Errorf("count of flag[-actions] and flag[-values] not equal") } v, err := strconv.Atoi(elements[0]) if err != nil { @@ -333,7 +333,7 @@ func (c *commandS3CircuitBreaker) initActionsAndValues(buckets, actions, limitTy } v, err := parseMBToBytes(elements[0]) if err != nil { - return nil, nil, nil, fmt.Errorf("value of -max must be a legal number(s)") + return nil, nil, nil, fmt.Errorf("value of -values must be a legal number(s)") } for range cmdActions { cmdValues = append(cmdValues, v) @@ -342,13 +342,13 @@ func (c *commandS3CircuitBreaker) initActionsAndValues(buckets, actions, limitTy for _, value := range elements { v, err := parseMBToBytes(value) if err != nil { - return nil, nil, nil, fmt.Errorf("value of -max must be a legal number(s)") + return nil, nil, nil, fmt.Errorf("value of -values must be a legal number(s)") } cmdValues = append(cmdValues, v) } } default: - return nil, nil, nil, fmt.Errorf("type not valid, only 'count' and 'bytes' are allowed") + return nil, nil, nil, fmt.Errorf("type not valid, only 'Count' and 'MB' are allowed") } } else { *limitType = "" diff --git a/weed/shell/command_s3_circuitbreaker_help_test.go b/weed/shell/command_s3_circuitbreaker_help_test.go new file mode 100644 index 0000000000..63fd215ca2 --- /dev/null +++ b/weed/shell/command_s3_circuitbreaker_help_test.go @@ -0,0 +1,53 @@ +package shell + +import ( + "bytes" + "strings" + "testing" +) + +// Every example in the s3.circuitBreaker help text has to run as written. +func TestCircuitBreakerHelpExamplesRun(t *testing.T) { + originalLoadConfig := LoadConfig + t.Cleanup(func() { LoadConfig = originalLoadConfig }) + LoadConfig = func(*CommandEnv, string, string, *bytes.Buffer) error { return nil } + + cmd := &commandS3CircuitBreaker{} + examples := 0 + for _, line := range strings.Split(cmd.Help(), "\n") { + fields := strings.Fields(line) + if len(fields) == 0 || fields[0] != cmd.Name() { + continue + } + examples++ + // Leave out -apply: it saves the result inside the filer. + var args []string + for _, f := range fields[1:] { + if f != "-apply" { + args = append(args, f) + } + } + var out bytes.Buffer + if err := cmd.Do(args, nil, &out); err != nil { + t.Errorf("%s: %v", strings.TrimSpace(line), err) + } + } + if examples == 0 { + t.Fatal("found no examples in the help text") + } +} + +func TestCircuitBreakerInvalidActionError(t *testing.T) { + originalLoadConfig := LoadConfig + t.Cleanup(func() { LoadConfig = originalLoadConfig }) + LoadConfig = func(*CommandEnv, string, string, *bytes.Buffer) error { return nil } + + cmd := &commandS3CircuitBreaker{} + err := cmd.Do([]string{"-global", "-type", "Count", "-actions", "Read,Bogus", "-values", "1,2"}, nil, &bytes.Buffer{}) + if err == nil { + t.Fatal("expected an error for an unknown action") + } + if msg := err.Error(); !strings.Contains(msg, "value(Bogus)") || !strings.Contains(msg, "flag[-actions]") { + t.Errorf("error should name the bad action and -actions: %v", err) + } +}