fix: surface plugin error in exec.go (#1228)

pkg/common/plugin/exec.go

@@ -18,7 +18,6 @@ package plugin
 import (
 	"bytes"
 	"context"
-	"encoding/json"
 	"fmt"
 	"io"
 	"os/exec"
@@ -46,6 +45,7 @@ type DefaultExecutor struct {
 	Stderr io.Writer
 }
 
+// return the command output and the error
 func (e *DefaultExecutor) ExecutePlugin(ctx context.Context, pluginPath string, cmdArgs []string, stdinData []byte, environ []string) ([]byte, error) {
 	stdout := &bytes.Buffer{}
 	stderr := &bytes.Buffer{}
@@ -83,6 +83,7 @@ func (e *DefaultExecutor) ExecutePlugin(ctx context.Context, pluginPath string,
 		// If the plugin is about to be completed, then we wait a
 		// second and try it again
 		if strings.Contains(err.Error(), "text file busy") {
+			logrus.Debugf("command returned text file busy, retrying after %v", waitDuration)
 			time.Sleep(waitDuration)
 			continue
 		}
@@ -102,17 +103,16 @@ func (e *DefaultExecutor) ExecutePlugin(ctx context.Context, pluginPath string,
 }
 
 func (e *DefaultExecutor) pluginErr(err error, stdout, stderr []byte) error {
-	errMsg := Error{}
-	if len(stdout) == 0 {
-		if len(stderr) == 0 {
-			errMsg.Msg = fmt.Sprintf("plugin failed with no proper error message: %v", err)
-		} else {
-			errMsg.Msg = fmt.Sprintf("plugin failed with error: %q", string(stderr))
-		}
-	} else if perr := json.Unmarshal(stdout, &errMsg); perr != nil {
-		errMsg.Msg = fmt.Sprintf("plugin failed and parsing its error message also failed with error %q: %v", string(stdout), perr)
-	}
-	return &errMsg
+	var stdOutBuffer bytes.Buffer
+	var stdErrBuffer bytes.Buffer
+
+	// writing them to buffer to avoid lint error
+	stdOutBuffer.Write(stdout)
+	stdErrBuffer.Write(stderr)
+
+	errCombined := Error{}
+	errCombined.Msg = fmt.Sprintf("plugin failed with error: '%v', msg from stError '%v', msg from stdOut '%v'", err.Error(), stdErrBuffer.String(), stdOutBuffer.String())
+	return &errCombined
 }
 
 func (e *DefaultExecutor) FindInPaths(plugin string, paths []string) (string, error) {

pkg/common/plugin/exec_test.go

@@ -0,0 +1,46 @@
+/*
+Copyright The Ratify Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package plugin
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestPluginErr(t *testing.T) {
+	stdOut := []byte("This is a string from std out")
+	stdErr := []byte("This is a string from std err")
+	errMsg := Error{}
+	errMsg.Msg = "plugin error"
+
+	e := DefaultExecutor{}
+	err := e.pluginErr(&errMsg, stdOut, stdErr)
+	if err == nil {
+		t.Fatalf("plugin error expected")
+	}
+
+	if !strings.Contains(err.Error(), errMsg.Msg) {
+		t.Fatalf("error msg should contain stdOut error msg, actual '%v'", err.Error())
+	}
+
+	if !strings.Contains(err.Error(), string(stdOut)) {
+		t.Fatalf("error msg should contain stdOut msg, actual '%v'", err.Error())
+	}
+
+	if !strings.Contains(err.Error(), string(stdErr)) {
+		t.Fatalf("error msg should contain stdErr msg, actual '%v'", err.Error())
+	}
+}