diff --git a/code/sonalyze/application/local.go b/code/sonalyze/application/local.go index 1d35d57d..78a7f82e 100644 --- a/code/sonalyze/application/local.go +++ b/code/sonalyze/application/local.go @@ -3,6 +3,7 @@ package application import ( + "errors" "fmt" "io" "math" @@ -19,17 +20,17 @@ import ( "sonalyze/sonarlog" ) -func LocalOperation(cmd SampleAnalysisCommand, _ io.Reader, stdout, stderr io.Writer) error { - args := cmd.SharedFlags() +func LocalOperation(command SampleAnalysisCommand, _ io.Reader, stdout, stderr io.Writer) error { + args := command.SharedFlags() - cfg, err := db.MaybeGetConfig(cmd.ConfigFile()) + cfg, err := db.MaybeGetConfig(command.ConfigFile()) if err != nil { return err } - hostGlobber, recordFilter, err := buildRecordFilters(cmd, cfg, args.Verbose) + hostGlobber, recordFilter, err := buildRecordFilters(command, cfg, args.Verbose) if err != nil { - return fmt.Errorf("Failed to create record filter\n%w", err) + return fmt.Errorf("Failed to create record filter: %v", err) } var theLog db.SampleCluster @@ -39,7 +40,7 @@ func LocalOperation(cmd SampleAnalysisCommand, _ io.Reader, stdout, stderr io.Wr theLog, err = db.OpenPersistentCluster(args.DataDir, cfg) } if err != nil { - return fmt.Errorf("Failed to open log store\n%w", err) + return fmt.Errorf("Failed to open log store: %v", err) } streams, bounds, read, dropped, err := @@ -49,11 +50,11 @@ func LocalOperation(cmd SampleAnalysisCommand, _ io.Reader, stdout, stderr io.Wr args.ToDate, hostGlobber, recordFilter, - cmd.NeedsBounds(), + command.NeedsBounds(), args.Verbose, ) if err != nil { - return fmt.Errorf("Failed to read log records\n%w", err) + return fmt.Errorf("Failed to read log records: %v", err) } if args.Verbose { Log.Infof("%d records read + %d dropped\n", read, dropped) @@ -61,27 +62,21 @@ func LocalOperation(cmd SampleAnalysisCommand, _ io.Reader, stdout, stderr io.Wr } sonarlog.ComputePerSampleFields(streams) - err = cmd.Perform(stdout, cfg, theLog, streams, bounds, hostGlobber, recordFilter) - - if err != nil { - return fmt.Errorf("Failed to perform operation\n%w", err) - } - - return nil + return command.Perform(stdout, cfg, theLog, streams, bounds, hostGlobber, recordFilter) } func buildRecordFilters( - cmd SampleAnalysisCommand, + command SampleAnalysisCommand, cfg *config.ClusterConfig, verbose bool, ) (*hostglob.HostGlobber, *db.SampleFilter, error) { - args := cmd.SharedFlags() + args := command.SharedFlags() // Temporary limitation. - if _, ok := cmd.(*profile.ProfileCommand); ok { + if _, ok := command.(*profile.ProfileCommand); ok { if len(args.RecordFilterArgs.Job) != 1 || len(args.RecordFilterArgs.ExcludeJob) != 0 { - return nil, nil, fmt.Errorf("Exactly one specific job number is required by `profile`") + return nil, nil, errors.New("Exactly one specific job number is required by `profile`") } } @@ -108,7 +103,7 @@ func buildRecordFilters( // Command-specific defaults for the record filters. - allUsers, skipSystemUsers, excludeSystemCommands, excludeHeartbeat := cmd.DefaultRecordFilters() + allUsers, skipSystemUsers, excludeSystemCommands, excludeHeartbeat := command.DefaultRecordFilters() // Included users, empty means "all" @@ -137,7 +132,7 @@ func buildRecordFilters( } else if name := os.Getenv("USER"); name != "" { includeUsers[StringToUstr(name)] = true } else { - return nil, nil, fmt.Errorf("Not able to determine user, none given and $LOGNAME and $USER are empty") + return nil, nil, errors.New("Not able to determine user, none given and $LOGNAME and $USER are empty") } } diff --git a/code/sonalyze/application/remote.go b/code/sonalyze/application/remote.go index bb318164..6530fccb 100644 --- a/code/sonalyze/application/remote.go +++ b/code/sonalyze/application/remote.go @@ -135,25 +135,33 @@ func RemoteOperation(rCmd RemotableCommand, verb string, stdin io.Reader, stdout } err = command.Run() + + // If there is a processing error on the remote end then the server will respond with a 400 code + // and the text that would otherwise go to stderr, see runSonalyze() in daemon/perform.go. That + // translates as a non-nil error with code 22 here, and the error message is on our local + // stdout. + // + // However if there is a problem with contacting the host or other curl failure, then the exit + // code is king and stderr may have some text. + // + // Curl has an elaborate set of exit codes, we could be more precise here but for most remote + // cases the user would just look them up. + if err != nil { - if rCmd.VerboseFlag() { - outs := newStdout.String() - if outs != "" { - fmt.Fprintf(stdout, "Output from failed (%s) subprocess: %s", err, outs) - } - errs := newStderr.String() - if errs != "" { - fmt.Fprintf(stdout, "Errors from failed (%s) subprocess: %s", err, errs) + if xe, ok := err.(*exec.ExitError); ok { + switch xe.ExitCode() { + case 22: + return fmt.Errorf("Remote: %s", newStdout.String()) + case 5, 6, 7: + return fmt.Errorf("Failed to resolve remote host (or proxy). Exit code %v, stderr=%s", + xe.ExitCode(), string(xe.Stderr)) + default: + return fmt.Errorf("Curl problem: exit code %v, stderr=%s", xe.ExitCode(), string(xe.Stderr)) } } - // Print this unredacted on the assumption that the remote sonalyzed/sonalyze don't - // reveal anything they shouldn't. - return err - } - errs := newStderr.String() - if errs != "" { - return errors.New(errs) + return fmt.Errorf("Could not start curl: %v", err) } + // print, not println, or we end up adding a blank line that confuses consumers fmt.Fprint(stdout, newStdout.String()) return nil diff --git a/code/sonalyze/cmd/args.go b/code/sonalyze/cmd/args.go index be87049d..441842df 100644 --- a/code/sonalyze/cmd/args.go +++ b/code/sonalyze/cmd/args.go @@ -148,7 +148,7 @@ func (ra *RemotingArgs) Add(fs *flag.FlagSet) { func (ra *RemotingArgs) Validate() error { if ra.Remote != "" || ra.Cluster != "" { if ra.Remote == "" || ra.Cluster == "" { - return fmt.Errorf("-remote and -cluster must be used together") + return errors.New("-remote and -cluster must be used together") } ra.Remoting = true } @@ -243,10 +243,10 @@ func (s *SourceArgs) Validate() error { // calling Validate(), it would confuse the matter - just disallow explicit values. (This // is a small abstraction leak.) if s.DataDir != "" { - return fmt.Errorf("-data-dir may not be used with -remote or -cluster") + return errors.New("-data-dir may not be used with -remote or -cluster") } if len(s.LogFiles) > 0 { - return fmt.Errorf("-- logfile ... may not be used with -remote or -cluster") + return errors.New("-- logfile ... may not be used with -remote or -cluster") } } else { // Compute and clean the dataDir and clean any logfiles. If we have neither logfiles nor @@ -260,7 +260,7 @@ func (s *SourceArgs) Validate() error { s.LogFiles[i] = path.Clean(s.LogFiles[i]) } } else if s.DataDir == "" { - return fmt.Errorf("Required -data-dir or -- logfile ...") + return errors.New("Required -data-dir or -- logfile ...") } } @@ -523,7 +523,7 @@ func ValidateFormatArgs( var others map[string]bool fa.PrintFields, others, err = ParseFormatSpec(defaultFields, fa.Fmt, formatters, aliases) if err == nil && len(fa.PrintFields) == 0 { - err = errors.New("No output fields were selected in format string") + err = errors.New("No valid output fields were selected in format string") } fa.PrintOpts = StandardFormatOptions(others, def) return err @@ -584,7 +584,7 @@ func (rs *RepeatableCommaSeparated[T]) Set(s string) error { ws := make([]T, 0, len(ys)) for _, y := range ys { if y == "" { - return errors.New("Empty string") + return errors.New("Empty string is an invalid argument") } n, err := rs.fromString(y) if err != nil { diff --git a/code/sonalyze/cmd/load/load.go b/code/sonalyze/cmd/load/load.go index 367e43a9..dca6ece0 100644 --- a/code/sonalyze/cmd/load/load.go +++ b/code/sonalyze/cmd/load/load.go @@ -141,7 +141,7 @@ func (lc *LoadCommand) Validate() error { var e4 error if lc.All && lc.Last { - e4 = errors.New("Incoherent printing options") + e4 = errors.New("Incoherent printing options: both -all and -last") } if !lc.All && !lc.Last { lc.All = true diff --git a/code/sonalyze/cmd/nodes/nodes.go b/code/sonalyze/cmd/nodes/nodes.go index 983246a6..9f1ce81c 100644 --- a/code/sonalyze/cmd/nodes/nodes.go +++ b/code/sonalyze/cmd/nodes/nodes.go @@ -113,7 +113,7 @@ func (nc *NodeCommand) Perform(_ io.Reader, stdout, stderr io.Writer) error { theLog, err = db.OpenPersistentCluster(nc.DataDir, cfg) } if err != nil { - return fmt.Errorf("Failed to open log store\n%w", err) + return fmt.Errorf("Failed to open log store: %v", err) } // Read and filter the raw sysinfo data. @@ -125,7 +125,7 @@ func (nc *NodeCommand) Perform(_ io.Reader, stdout, stderr io.Writer) error { nc.Verbose, ) if err != nil { - return fmt.Errorf("Failed to read log records\n%w", err) + return fmt.Errorf("Failed to read log records: %v", err) } records := uslices.Catenate(recordBlobs) if nc.Verbose { @@ -135,7 +135,7 @@ func (nc *NodeCommand) Perform(_ io.Reader, stdout, stderr io.Writer) error { hostGlobber, recordFilter, err := nc.buildRecordFilter(nc.Verbose) if err != nil { - return fmt.Errorf("Failed to create record filter\n%w", err) + return fmt.Errorf("Failed to create record filter: %v", err) } records = slices.DeleteFunc(records, func(s *config.NodeConfigRecord) bool { diff --git a/code/sonalyze/cmd/profile/perform.go b/code/sonalyze/cmd/profile/perform.go index 647f3200..a339e39d 100644 --- a/code/sonalyze/cmd/profile/perform.go +++ b/code/sonalyze/cmd/profile/perform.go @@ -3,6 +3,7 @@ package profile import ( "cmp" "errors" + "fmt" "io" "math" "slices" @@ -33,7 +34,7 @@ func (pc *ProfileCommand) Perform( jobId := pc.Job[0] if len(streams) == 0 { - return errors.New("No processes") + return fmt.Errorf("No processes matching job ID(s): %v", pc.Job) } // Simplify: Assert that the input has only a single host. diff --git a/code/sonalyze/cmd/profile/print.go b/code/sonalyze/cmd/profile/print.go index a7e66e7e..a4f1fb88 100644 --- a/code/sonalyze/cmd/profile/print.go +++ b/code/sonalyze/cmd/profile/print.go @@ -326,12 +326,12 @@ func lookupSingleFormatter( formatter = formatGpuMem n++ default: - err = fmt.Errorf("Not a known field: %s", f.Name) + err = fmt.Errorf("Not a printable field for this output format: %s", f.Name) return } } if n != 1 { - err = errors.New("formatted output needs exactly one valid field") + err = errors.New("Formatted output needs exactly one valid field") } return } diff --git a/code/sonalyze/cmd/profile/profile.go b/code/sonalyze/cmd/profile/profile.go index 1e2fd305..199dfcf0 100644 --- a/code/sonalyze/cmd/profile/profile.go +++ b/code/sonalyze/cmd/profile/profile.go @@ -70,7 +70,7 @@ func (pc *ProfileCommand) Validate() error { profileAliases, ) if e4 == nil && len(pc.PrintFields) == 0 && !others["json"] { - e4 = errors.New("No output fields were selected in format string") + e4 = errors.New("No valid output fields were selected in format string") } // Options for profile are restrictive, and a little wonky because html is handled on the side, @@ -86,7 +86,7 @@ func (pc *ProfileCommand) Validate() error { var e5 error if pc.htmlOutput && !pc.PrintOpts.IsDefaultFormat() { - e5 = errors.New("Multiple formats requested") + e5 = errors.New("Multiple output formats requested") } // The printing code uses custom logic for everything but Fixed layout, and the custom logic diff --git a/code/sonalyze/cmd/report/report.go b/code/sonalyze/cmd/report/report.go index be0a7e6d..a59a0c9e 100644 --- a/code/sonalyze/cmd/report/report.go +++ b/code/sonalyze/cmd/report/report.go @@ -75,7 +75,7 @@ func (rc *ReportCommand) Validate() error { // combination of fs.ValidPath + filepath.Localize is just as good and more permissive, it would // allow for files in subdirectories of the report directory too. if !filenameRe.MatchString(rc.ReportName) || !filepath.IsLocal(rc.ReportName) { - return errors.New("Illegal report file name") + return errors.New("Illegal file name for -report-name") } if rc.ReportDir == "" { diff --git a/code/sonalyze/cmd/sacct/perform.go b/code/sonalyze/cmd/sacct/perform.go index 4c938f51..9607ba8a 100644 --- a/code/sonalyze/cmd/sacct/perform.go +++ b/code/sonalyze/cmd/sacct/perform.go @@ -35,7 +35,7 @@ func (sc *SacctCommand) Perform(_ io.Reader, stdout, stderr io.Writer) error { theLog, err = db.OpenPersistentCluster(sc.DataDir, cfg) } if err != nil { - return fmt.Errorf("Failed to open log store\n%w", err) + return fmt.Errorf("Failed to open log store: %v", err) } // Read the raw sacct data. @@ -46,7 +46,7 @@ func (sc *SacctCommand) Perform(_ io.Reader, stdout, stderr io.Writer) error { sc.Verbose, ) if err != nil { - return fmt.Errorf("Failed to read log records\n%w", err) + return fmt.Errorf("Failed to read log records: %v", err) } // TODO: The catenation is expedient, we should be looping over the nested set (or in the // future, using an iterator). diff --git a/code/sonalyze/cmd/top/top.go b/code/sonalyze/cmd/top/top.go index 00efe691..c7cd0eef 100644 --- a/code/sonalyze/cmd/top/top.go +++ b/code/sonalyze/cmd/top/top.go @@ -112,7 +112,7 @@ func (tc *TopCommand) Perform(stdin io.Reader, stdout, stderr io.Writer) error { theLog, err = db.OpenPersistentCluster(tc.DataDir, cfg) } if err != nil { - return fmt.Errorf("Failed to open log store\n%w", err) + return fmt.Errorf("Failed to open log store: %v", err) } streams, _, read, dropped, err := @@ -124,7 +124,7 @@ func (tc *TopCommand) Perform(stdin io.Reader, stdout, stderr io.Writer) error { tc.Verbose, ) if err != nil { - return fmt.Errorf("Failed to read log records\n%w", err) + return fmt.Errorf("Failed to read log records: %v", err) } if tc.Verbose { Log.Infof("%d records read + %d dropped\n", read, dropped) diff --git a/code/sonalyze/cmd/uptime/uptime.go b/code/sonalyze/cmd/uptime/uptime.go index 1e75677a..df06f857 100644 --- a/code/sonalyze/cmd/uptime/uptime.go +++ b/code/sonalyze/cmd/uptime/uptime.go @@ -37,7 +37,7 @@ func (uc *UptimeCommand) Add(fs *flag.FlagSet) { uc.FormatArgs.Add(fs) fs.UintVar(&uc.Interval, "interval", 0, - "The maximum sampling `interval` in minutes (before any randomization) seen in the data") + "The maximum sampling `interval` in minutes (before any randomization) seen in the data (required)") fs.BoolVar(&uc.OnlyUp, "only-up", false, "Show only times when systems are up") fs.BoolVar(&uc.OnlyDown, "only-down", false, "Show only times when systems are down") } diff --git a/code/sonalyze/daemon/daemon.go b/code/sonalyze/daemon/daemon.go index 5c2d27ab..3cc9a3fe 100644 --- a/code/sonalyze/daemon/daemon.go +++ b/code/sonalyze/daemon/daemon.go @@ -155,13 +155,13 @@ func (dc *DaemonCommand) Validate() error { if dc.getAuthFile != "" { dc.getAuthenticator, e4 = auth.ReadPasswords(dc.getAuthFile) if e4 != nil { - e4 = fmt.Errorf("Failed to read analysis authentication file %w", e4) + e4 = fmt.Errorf("Failed to read analysis authentication file: %v", e4) } } if dc.postAuthFile != "" { dc.postAuthenticator, e5 = auth.ReadPasswords(dc.postAuthFile) if e5 != nil { - return fmt.Errorf("Failed to read upload authentication file %w", e5) + return fmt.Errorf("Failed to read upload authentication file: %v", e5) } } _, dc.aliasResolver, e6 = db.ReadClusterData(dc.jobanalyzerDir) diff --git a/code/sonalyze/daemon/perform.go b/code/sonalyze/daemon/perform.go index 4d2a8fd9..7467f1f4 100644 --- a/code/sonalyze/daemon/perform.go +++ b/code/sonalyze/daemon/perform.go @@ -40,7 +40,7 @@ import ( func (dc *DaemonCommand) RunDaemon(_ io.Reader, _, stderr io.Writer) error { logger, err := syslog.Dial("", "", syslog.LOG_INFO|syslog.LOG_USER, logTag) if err != nil { - return fmt.Errorf("FATAL ERROR: Failing to open logger: %w", err) + return fmt.Errorf("FATAL ERROR: Failing to open logger: %v", err) } Log.SetUnderlying(logger) diff --git a/code/sonalyze/db/clusterstore.go b/code/sonalyze/db/clusterstore.go index 3e2d4ca7..782ab1ce 100644 --- a/code/sonalyze/db/clusterstore.go +++ b/code/sonalyze/db/clusterstore.go @@ -480,7 +480,7 @@ func readRecordsFromFiles[T any]( if bad != "" { recordBlobs = nil - err = fmt.Errorf("Failed to process one or more files:\n%s", bad) + err = fmt.Errorf("Failed to process one or more files: %s", bad) return } diff --git a/code/sonalyze/db/logfile.go b/code/sonalyze/db/logfile.go index 1991e72a..f7cabadf 100644 --- a/code/sonalyze/db/logfile.go +++ b/code/sonalyze/db/logfile.go @@ -254,7 +254,7 @@ func (lf *LogFile) flushSyncLocked() (err error) { // // Could also be too many open files, in which case we really want to close all open // files and retry. - err = fmt.Errorf("Failed to open/create file (%v)", err) + err = fmt.Errorf("Failed to open/create file: %v", err) return } defer f.Close() diff --git a/code/sonalyze/sonalyze.go b/code/sonalyze/sonalyze.go index c9792550..97e16e5f 100644 --- a/code/sonalyze/sonalyze.go +++ b/code/sonalyze/sonalyze.go @@ -50,7 +50,7 @@ var stdhandler = standardCommandLineHandler{} func main() { err := sonalyze() if err != nil { - fmt.Fprintln(os.Stderr, err) + fmt.Fprintf(os.Stderr, "Sonalyze failed: %v\n", err) os.Exit(1) } } @@ -85,7 +85,7 @@ func sonalyze() error { default: anyCmd, verb := stdhandler.ParseVerb(cmdName, maybeVerb) if anyCmd == nil { - fmt.Fprintf(out, "Required operation missing, try `sonalyze help`\n") + fmt.Fprintf(out, "Unknown operation: %s\nTry `sonalyze help`\n", maybeVerb) os.Exit(2) } @@ -114,7 +114,7 @@ func sonalyze() error { err := stdhandler.ParseArgs(verb, args, anyCmd, fs) if err != nil { - fmt.Fprint(out, err.Error()) + fmt.Fprintf(out, "Bad arguments: %v\nTry `sonalyze %s -h`\n", err, maybeVerb) os.Exit(2) } @@ -193,7 +193,7 @@ func (_ *standardCommandLineHandler) ParseArgs( if lfCmd, ok := command.(cmd.SetRestArgumentsAPI); ok { lfCmd.SetRestArguments(rest) } else { - return fmt.Errorf("Rest arguments not accepted by `%s`.\n", verb) + return fmt.Errorf("Rest arguments not accepted by `%s`", verb) } } @@ -205,12 +205,7 @@ func (_ *standardCommandLineHandler) ParseArgs( return nil } - err = command.Validate() - if err != nil { - return fmt.Errorf("Bad arguments, try -h\n%w\n", err) - } - - return nil + return command.Validate() } func (_ *standardCommandLineHandler) StartCPUProfile(profileFile string) (func(), error) { @@ -220,7 +215,7 @@ func (_ *standardCommandLineHandler) StartCPUProfile(profileFile string) (func() f, err := os.Create(profileFile) if err != nil { - return nil, fmt.Errorf("Failed to create profile\n%w", err) + return nil, fmt.Errorf("Failed to create profile: %v", err) } pprof.StartCPUProfile(f) @@ -270,7 +265,7 @@ func (_ *daemonCommandLineHandler) ParseArgs( return err } if command.CpuProfileFile() != "" { - return fmt.Errorf("The -cpuprofile cannot be run remotely") + return errors.New("The -cpuprofile cannot be run remotely") } return nil }