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.
Snowflake-Labs · Oct 18, 2022 · ef67dd3 · ef67dd3
1 parent 5b3121c
commit ef67dd3
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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)