Skip to content

Commit

Permalink
fix plugin error propagation (#830)
Browse files Browse the repository at this point in the history
#### What this PR does / why we need it:

- If the plugin is called via `go run` (like in the tests), this command
adds an implicit error status line, which
  must be handled in the plugin adapter to find the correct error json.
- The plugin might provide error output. This error output must be
separated from the formal error status
- As long as we don't have a formal way to get to the stderr stream from
the context, the error output is added
  to the error message
  • Loading branch information
mandelsoft authored Jun 28, 2024
1 parent 2361b54 commit c71aed4
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 8 deletions.
15 changes: 15 additions & 0 deletions cmds/subcmdplugin/cmds/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ var _ = Describe("subcmdplugin", func() {
env.Cleanup()
})

Context("error handling", func() {
It("provides error", func() {
var buf bytes.Buffer

ExpectError(env.CatchOutput(&buf).Execute("group", "demo", "--version=error")).To(MatchError("error processing plugin command command: demo error"))
})

It("provides error and error outputp", func() {
var buf bytes.Buffer

ExpectError(env.CatchOutput(&buf).Execute("group", "demo", "--version=error this is an error my friend")).To(MatchError(`error processing plugin command command: demo error: with stderr
this is an error my friend`))
})
})

Context("local help", func() {
It("shows group command help", func() {
var buf bytes.Buffer
Expand Down
9 changes: 9 additions & 0 deletions cmds/subcmdplugin/cmds/demo/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package demo

import (
"fmt"
"os"
"strings"

// bind OCM configuration.
_ "github.com/open-component-model/ocm/pkg/contexts/ocm/plugin/ppi/config"
Expand Down Expand Up @@ -30,5 +32,12 @@ type command struct {

func (c *command) Run(cmd *cobra.Command, args []string) error {
fmt.Printf("demo command called with arguments %v (and version option %s)\n", args, c.version)
if strings.HasPrefix(c.version, "error") {
msg := strings.TrimSpace(c.version[5:])
if len(msg) != 0 {
fmt.Fprintf(os.Stderr, "this is an error my friend\n")
}
return fmt.Errorf("demo error")
}
return nil
}
32 changes: 25 additions & 7 deletions pkg/contexts/ocm/plugin/cache/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"fmt"
"io"
"os/exec"
"strings"

"github.com/mandelsoft/goutils/errors"

"github.com/open-component-model/ocm/pkg/common/accessio"
"github.com/open-component-model/ocm/pkg/contexts/ocm/plugin/ppi/cmds"
Expand All @@ -29,18 +32,33 @@ func Exec(execpath string, config json.RawMessage, r io.Reader, w io.Writer, arg
err := cmd.Run()
if err != nil {
var result cmds.Error
var msg string
data := stderr.Bytes()
var cerr error
data := strings.TrimSpace(string(stderr.Bytes()))
if len(data) == 0 {
msg = err.Error()
cerr = errors.New(err.Error())
} else {
if err := json.Unmarshal(stderr.Bytes(), &result); err == nil {
msg = result.Error
// handle implicit error output from go run
i := strings.LastIndex(data, "\n")
for i > 0 && (i == len(data)-1 || strings.HasPrefix(data[i+1:], "exit status")) {
data = data[:i]
i = strings.LastIndex(data, "\n")
}
if err := json.Unmarshal([]byte(data), &result); err == nil {
cerr = errors.New(result.Error)
} else {
msg = fmt.Sprintf("[%s]", string(stderr.Bytes()))
if err := json.Unmarshal([]byte(data[i+1:]), &result); err == nil {
cerr = errors.New(result.Error)
// TODO pass effective stderr from CLI
data = strings.TrimSpace(data[:i])
if len(data) > 0 {
cerr = fmt.Errorf("%w: with stderr\n%s", cerr, data)
}
} else {
cerr = fmt.Errorf("[%s]", data)
}
}
}
return nil, fmt.Errorf("%s", msg)
return nil, cerr
}
if l, ok := stdout.(*accessio.LimitedBuffer); ok {
if l.Exceeded() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ func (p *pluginImpl) Command(name string, reader io.Reader, writer io.Writer, cm
return errors.ErrNotFound("command", name)
}

defer finalize.FinalizeWithErrorPropagationf(&rerr, "error processing plugin command call %s", name)
defer finalize.FinalizeWithErrorPropagation(&rerr)

var f vfs.File

Expand Down

0 comments on commit c71aed4

Please sign in to comment.