From ef67dd354af729dd957597d51b30e2a61c59f270 Mon Sep 17 00:00:00 2001 From: James Chacon Date: Tue, 18 Oct 2022 16:50:38 -0700 Subject: [PATCH] Make error reporting from a subprocess more consistent. (#173) i.e. print stderr on actual errors. For packages combine stdout/stderr together so it's not lost for debugging. --- services/fdb/server/fdbcli.go | 7 ++----- services/packages/server/packages.go | 13 +++++-------- services/process/server/process.go | 2 +- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/services/fdb/server/fdbcli.go b/services/fdb/server/fdbcli.go index a383ce6f..c641afe2 100644 --- a/services/fdb/server/fdbcli.go +++ b/services/fdb/server/fdbcli.go @@ -234,11 +234,8 @@ func (s *server) FDBCLI(req *pb.FDBCLIRequest, stream pb.CLI_FDBCLIServer) error if err != nil { return status.Errorf(codes.Internal, "error running fdbcli cmd (%+v): %v", command, err) } - if run.Error != nil { - return status.Errorf(codes.Internal, "can't exec fdbcli: %v - %s", run.Error, run.Stderr) - } - if run.ExitCode != 0 { - return status.Errorf(codes.Internal, "fdbcli returned non-zero: %d - %v - %s", run.ExitCode, run.Error, run.Stderr) + if err := run.Error; run.ExitCode != 0 || err != nil { + return status.Errorf(codes.Internal, "error from running - %v\nstdout:\n%s\nstderr:\n%s", err, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String())) } // Make sure we always remove logs even if errors happen below. diff --git a/services/packages/server/packages.go b/services/packages/server/packages.go index d92af0b2..4d110d79 100644 --- a/services/packages/server/packages.go +++ b/services/packages/server/packages.go @@ -196,14 +196,12 @@ func (s *server) Install(ctx context.Context, req *pb.InstallRequest) (*pb.Insta if err != nil { return nil, err } - if err := run.Error; run.ExitCode != 0 || err != nil { return nil, status.Errorf(codes.Internal, "error from running - %v\nstdout:\n%s\nstderr:\n%s", err, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String())) } - // This may return stderr output about repos but unless return code was non-zero we don't care. return &pb.InstallReply{ - DebugOutput: run.Stdout.String(), + DebugOutput: fmt.Sprintf("%s%s", run.Stdout.String(), run.Stderr.String()), }, nil } @@ -247,7 +245,7 @@ func (s *server) Update(ctx context.Context, req *pb.UpdateRequest) (*pb.UpdateR return nil, err } if err := run.Error; run.ExitCode != 0 || err != nil { - return nil, status.Errorf(codes.Internal, "package %s at version %s doesn't appear to be installed.\nStderr:\n%s", req.Name, req.OldVersion, util.TrimString(run.Stderr.String())) + return nil, status.Errorf(codes.Internal, "package %s at version %s doesn't appear to be installed.\nstdout:\n%s\nstderr:\n%s", req.Name, req.OldVersion, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String())) } // A 0 return means we're ok to proceed. @@ -256,12 +254,11 @@ func (s *server) Update(ctx context.Context, req *pb.UpdateRequest) (*pb.UpdateR return nil, err } if err := run.Error; run.ExitCode != 0 || err != nil { - return nil, status.Errorf(codes.Internal, "error from running %q: %v", updateCommand, err) + return nil, status.Errorf(codes.Internal, "error from running - %v\nstdout:\n%s\nstderr:\n%s", err, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String())) } - // This may return stderr output about repos but unless return code was non-zero we don't care. return &pb.UpdateReply{ - DebugOutput: run.Stdout.String(), + DebugOutput: fmt.Sprintf("%s%s", run.Stdout.String(), run.Stderr.String()), }, nil } @@ -352,7 +349,7 @@ func (s *server) ListInstalled(ctx context.Context, req *pb.ListInstalledRequest return nil, err } if err := run.Error; run.ExitCode != 0 || err != nil { - return nil, status.Errorf(codes.Internal, "error from running %q: %v", command, err) + return nil, status.Errorf(codes.Internal, "error from running - %v\nstdout:\n%s\nstderr:\n%s", err, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String())) } return parseListInstallOutput(req.PackageSystem, run.Stdout) diff --git a/services/process/server/process.go b/services/process/server/process.go index cd7af852..a33d5210 100644 --- a/services/process/server/process.go +++ b/services/process/server/process.go @@ -121,7 +121,7 @@ func (s *server) List(ctx context.Context, req *pb.ListRequest) (*pb.ListReply, } if err := run.Error; run.ExitCode != 0 || err != nil { - return nil, status.Errorf(codes.Internal, "command exited with error/non-zero exit: %v (%d)\n%s", err, run.ExitCode, util.TrimString(run.Stderr.String())) + return nil, status.Errorf(codes.Internal, "error from running - %v\nstdout:\n%s\nstderr:\n%s", err, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String())) } entries, err := parser(run.Stdout)