diff --git a/cliext/flags.gen.go b/cliext/flags.gen.go index 28668a3ed..c607e4c13 100644 --- a/cliext/flags.gen.go +++ b/cliext/flags.gen.go @@ -38,8 +38,8 @@ func (v *CommonOptions) BuildFlags(f *pflag.FlagSet) { f.StringVar(&v.Profile, "profile", "", "Profile to use for config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigFile, "disable-config-file", false, "If set, disables loading environment config from config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigEnv, "disable-config-env", false, "If set, disables loading environment config from environment variables. EXPERIMENTAL.") - v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "info") - f.Var(&v.LogLevel, "log-level", "Log level. Default is \"info\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.") + v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "never") + f.Var(&v.LogLevel, "log-level", "Log level. Default is \"never\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.") v.LogFormat = NewFlagStringEnum([]string{"text", "json", "pretty"}, "text") f.Var(&v.LogFormat, "log-format", "Log format. Accepted values: text, json.") v.Output = NewFlagStringEnum([]string{"text", "json", "jsonl", "none"}, "text") diff --git a/cliext/option-sets.yaml b/cliext/option-sets.yaml index c8472fd03..e7a7ff8cf 100644 --- a/cliext/option-sets.yaml +++ b/cliext/option-sets.yaml @@ -51,8 +51,8 @@ option-sets: - never description: | Log level. - Default is "info" for most commands and "warn" for "server start-dev". - default: info + Default is "never" for most commands and "warn" for "server start-dev". + default: never - name: log-format type: string-enum description: Log format. diff --git a/internal/temporalcli/commands.env.go b/internal/temporalcli/commands.env.go index d98c06f04..5d43843d7 100644 --- a/internal/temporalcli/commands.env.go +++ b/internal/temporalcli/commands.env.go @@ -13,7 +13,7 @@ import ( func (c *TemporalEnvCommand) envNameAndKey(cctx *CommandContext, args []string, keyFlag string) (string, string, error) { if len(args) > 0 { - cctx.Logger.Warn("Arguments to env commands are deprecated; please use --env and --key (or -k) instead") + fmt.Fprintln(cctx.Options.Stderr, "Warning: Arguments to env commands are deprecated; please use --env and --key (or -k) instead") if c.Parent.Env != "default" || keyFlag != "" { return "", "", fmt.Errorf("cannot specify both an argument and flags; please use flags instead") diff --git a/internal/temporalcli/commands.env_test.go b/internal/temporalcli/commands.env_test.go index cdd1157a9..1a2a0a45a 100644 --- a/internal/temporalcli/commands.env_test.go +++ b/internal/temporalcli/commands.env_test.go @@ -2,6 +2,7 @@ package temporalcli_test import ( "os" + "strings" "testing" "gopkg.in/yaml.v3" @@ -104,3 +105,53 @@ func TestEnv_InputValidation(t *testing.T) { res = h.Execute("env", "set", "myenv1.foo") h.ErrorContains(res.Err, `no value provided`) } + +func TestEnv_DeprecationWarningBypassesLogger(t *testing.T) { + h := NewCommandHarness(t) + defer h.Close() + + tmpFile, err := os.CreateTemp("", "") + h.NoError(err) + h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name() + defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile) + res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") + h.NoError(res.Err) + + // Using deprecated argument syntax should produce a warning on stderr. + // The warning must bypass the logger so it appears regardless of log level. + for _, logLevel := range []string{"never", "error", "info"} { + t.Run("log-level="+logLevel, func(t *testing.T) { + res = h.Execute("env", "get", "--log-level", logLevel, "myenv1") + h.NoError(res.Err) + + stderr := res.Stderr.String() + h.Contains(stderr, "Warning:") + h.Contains(stderr, "deprecated") + + // Must be a plain-text warning, not a structured log message + h.False(strings.Contains(stderr, "time="), "warning should not be a structured log message") + h.False(strings.Contains(stderr, "level="), "warning should not be a structured log message") + }) + } +} + +func TestEnv_DefaultLogLevelProducesNoLogOutput(t *testing.T) { + h := NewCommandHarness(t) + defer h.Close() + + tmpFile, err := os.CreateTemp("", "") + h.NoError(err) + h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name() + defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile) + + // env set logs "Setting env property" via cctx.Logger.Info(). With the + // default log level ("never"), this should not appear on stderr. + res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") + h.NoError(res.Err) + h.Empty(res.Stderr.String(), "default log level should produce no log output") + + // With --log-level info, the logger output should appear. + res = h.Execute("env", "set", "--env", "myenv1", "-k", "baz", "-v", "qux", "--log-level", "info") + h.NoError(res.Err) + h.Contains(res.Stderr.String(), "Setting env property") +} diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index a60307b83..1983ebdfa 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -143,11 +143,7 @@ func (c *CommandContext) preprocessOptions() error { if c.Err() != nil { err = fmt.Errorf("program interrupted") } - if c.Logger != nil { - c.Logger.Error(err.Error()) - } else { - fmt.Fprintln(os.Stderr, err) - } + fmt.Fprintf(c.Options.Stderr, "Error: %v\n", err) os.Exit(1) } } diff --git a/internal/temporalcli/commands.operator_namespace.go b/internal/temporalcli/commands.operator_namespace.go index ebcc96cab..ca4b6c6d5 100644 --- a/internal/temporalcli/commands.operator_namespace.go +++ b/internal/temporalcli/commands.operator_namespace.go @@ -19,7 +19,7 @@ func (c *TemporalOperatorCommand) getNSFromFlagOrArg0(cctx *CommandContext, args } if len(args) > 0 { - cctx.Logger.Warn("Passing the namespace as an argument is now deprecated; please switch to using -n instead") + fmt.Fprintln(cctx.Options.Stderr, "Warning: Passing the namespace as an argument is now deprecated; please switch to using -n instead") return args[0], nil } return c.Namespace, nil diff --git a/internal/temporalcli/commands.workflow_exec.go b/internal/temporalcli/commands.workflow_exec.go index d5ba34899..935504b96 100644 --- a/internal/temporalcli/commands.workflow_exec.go +++ b/internal/temporalcli/commands.workflow_exec.go @@ -94,7 +94,7 @@ func (c *TemporalWorkflowExecuteCommand) run(cctx *CommandContext, args []string // Log print failure and return workflow failure if workflow failed if closeEvent.EventType != enums.EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED { if err != nil { - cctx.Logger.Error("Workflow failed, and printing the output also failed", "error", err) + fmt.Fprintf(cctx.Options.Stderr, "Warning: printing workflow output failed: %v\n", err) } err = fmt.Errorf("workflow failed") } diff --git a/internal/temporalcli/commands.workflow_view.go b/internal/temporalcli/commands.workflow_view.go index 08fb5c76e..a63ca06ec 100644 --- a/internal/temporalcli/commands.workflow_view.go +++ b/internal/temporalcli/commands.workflow_view.go @@ -560,7 +560,7 @@ func (c *TemporalWorkflowResultCommand) run(cctx *CommandContext, _ []string) er // Log print failure and return workflow failure if workflow failed if closeEvent.EventType != enums.EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED { if err != nil { - cctx.Logger.Error("Workflow failed, and printing the output also failed", "error", err) + fmt.Fprintf(cctx.Options.Stderr, "Warning: printing workflow output failed: %v\n", err) } err = fmt.Errorf("workflow failed") } diff --git a/internal/temporalcli/commands_test.go b/internal/temporalcli/commands_test.go index 2312e442e..749f2c3e6 100644 --- a/internal/temporalcli/commands_test.go +++ b/internal/temporalcli/commands_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "log/slog" + "net" "regexp" "slices" "strings" @@ -625,6 +626,30 @@ func TestUnknownCommandExitsNonzero(t *testing.T) { assert.Contains(t, res.Err.Error(), "unknown command") } +func TestErrorReporting_IndependentOfLogLevel(t *testing.T) { + // Errors must be reported through Fail regardless of --log-level, and + // must not produce structured log output on stderr. + for _, logLevel := range []string{"never", "error", "info"} { + t.Run("log-level="+logLevel, func(t *testing.T) { + h := NewCommandHarness(t) + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + ln.Close() // close immediately so connection is refused + + res := h.Execute( + "workflow", "list", + "--address", ln.Addr().String(), + "--log-level", logLevel, + ) + require.Error(t, res.Err) + + stderr := res.Stderr.String() + assert.NotContains(t, stderr, "level=ERROR", + "errors should not appear as structured log messages on stderr") + }) + } +} + func (s *SharedServerSuite) TestHiddenAliasLogFormat() { _ = s.waitActivityStarted().GetID() res := s.Execute("workflow", "list", "--log-format", "pretty", "--address", s.Address())