Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

archive: improve filter error reporting #2025

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions pkg/archive/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ func getFilterPath(name string) string {
return path.(string)
}

type errorRecordingReader struct {
r io.Reader
err error
}

func (r *errorRecordingReader) Read(p []byte) (int, error) {
n, err := r.r.Read(p)
if r.err == nil && err != io.EOF {
r.err = err
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should prefer the first error, not the last one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that is better, changed now

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… but still not store err == nil?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… never mind, this works without an explicit check for err != nil correctly.

}
return n, err
}

// tryProcFilter tries to run the command specified in args, passing input to its stdin and returning its stdout.
// cleanup() is a caller provided function that will be called when the command finishes running, regardless of
// whether it succeeds or fails.
Expand All @@ -38,22 +51,20 @@ func tryProcFilter(args []string, input io.Reader, cleanup func()) (io.ReadClose

var stderrBuf bytes.Buffer

inputWithError := &errorRecordingReader{r: input}

r, w := io.Pipe()
cmd := exec.Command(path, args[1:]...)
cmd.Stdin = input
cmd.Stdin = inputWithError
cmd.Stdout = w
cmd.Stderr = &stderrBuf
go func() {
err := cmd.Run()
if err != nil && stderrBuf.Len() > 0 {
b := make([]byte, 1)
// if there is an error reading from input, prefer to return that error
_, errRead := input.Read(b)
if errRead != nil && errRead != io.EOF {
err = errRead
} else {
err = fmt.Errorf("%s: %w", strings.TrimRight(stderrBuf.String(), "\n"), err)
}
// if there is an error reading from input, prefer to return that error
if inputWithError.err != nil {
err = inputWithError.err
} else if err != nil && stderrBuf.Len() > 0 {
err = fmt.Errorf("%s: %w", strings.TrimRight(stderrBuf.String(), "\n"), err)
}
w.CloseWithError(err) // CloseWithErr(nil) == Close()
cleanup()
Expand Down