Merge pull request #5143 from cli/pager-epipe-ignore

Ignore EPIPE errors when writing to a closed pager

cmd/gh/main.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cli/cli/v2/pkg/cmd/factory"
 	"github.com/cli/cli/v2/pkg/cmd/root"
 	"github.com/cli/cli/v2/pkg/cmdutil"
+	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/utils"
 	"github.com/cli/safeexec"
 	"github.com/mattn/go-colorable"
@@ -201,6 +202,7 @@ func mainRun() exitCode {
 	rootCmd.SetArgs(expandedArgs)
 
 	if cmd, err := rootCmd.ExecuteC(); err != nil {
+		var pagerPipeError *iostreams.ErrClosedPagerPipe
 		if err == cmdutil.SilentError {
 			return exitError
 		} else if cmdutil.IsUserCancellation(err) {
@@ -211,6 +213,9 @@ func mainRun() exitCode {
 			return exitCancel
 		} else if errors.Is(err, authError) {
 			return exitAuth
+		} else if errors.As(err, &pagerPipeError) {
+			// ignore the error raised when piping to a closed pager
+			return exitOK
 		}
 
 		printError(stderr, err, cmd, hasDebug)

pkg/cmd/api/api.go

@@ -3,7 +3,6 @@ package api
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -13,7 +12,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"syscall"
 	"time"
 
 	"github.com/MakeNowJust/heredoc"
@@ -384,11 +382,7 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream
 		_, err = io.Copy(opts.IO.Out, responseBody)
 	}
 	if err != nil {
-		if errors.Is(err, syscall.EPIPE) {
-			err = nil
-		} else {
-			return
-		}
+		return
 	}
 
 	if serverError == "" && resp.StatusCode > 299 {

pkg/cmd/pr/diff/diff.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"strings"
-	"syscall"
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/v2/api"
@@ -112,9 +111,6 @@ func diffRun(opts *DiffOptions) error {
 
 	if !opts.UseColor {
 		_, err = io.Copy(opts.IO.Out, diff)
-		if errors.Is(err, syscall.EPIPE) {
-			return nil
-		}
 		return err
 	}
 

pkg/cmd/repo/view/view.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
-	"syscall"
 	"text/template"
 
 	"github.com/MakeNowJust/heredoc"
@@ -213,12 +212,7 @@ func viewRun(opts *ViewOptions) error {
 		View:        cs.Gray(fmt.Sprintf("View this repository on GitHub: %s", openURL)),
 	}
 
-	err = tmpl.Execute(stdout, repoData)
-	if err != nil && !errors.Is(err, syscall.EPIPE) {
-		return err
-	}
-
-	return nil
+	return tmpl.Execute(stdout, repoData)
 }
 
 func isMarkdownFile(filename string) bool {

pkg/iostreams/epipe_other.go

@@ -0,0 +1,13 @@
+//go:build !windows
+// +build !windows
+
+package iostreams
+
+import (
+	"errors"
+	"syscall"
+)
+
+func isEpipeError(err error) bool {
+	return errors.Is(err, syscall.EPIPE)
+}

pkg/iostreams/epipe_windows.go

@@ -0,0 +1,11 @@
+package iostreams
+
+import (
+	"errors"
+	"syscall"
+)
+
+func isEpipeError(err error) bool {
+	// 232 is Windows error code ERROR_NO_DATA, "The pipe is being closed".
+	return errors.Is(err, syscall.Errno(232))
+}

pkg/iostreams/iostreams.go

@@ -24,6 +24,11 @@ import (
 
 const DefaultWidth = 80
 
+// ErrClosedPagerPipe is the error returned when writing to a pager that has been closed.
+type ErrClosedPagerPipe struct {
+	error
+}
+
 type IOStreams struct {
 	In     io.ReadCloser
 	Out    io.Writer
@@ -197,7 +202,7 @@ func (s *IOStreams) StartPager() error {
 	if err != nil {
 		return err
 	}
-	s.Out = pagedOut
+	s.Out = &pagerWriter{pagedOut}
 	err = pagerCmd.Start()
 	if err != nil {
 		return err
@@ -211,7 +216,7 @@ func (s *IOStreams) StopPager() {
 		return
 	}
 
-	_ = s.Out.(io.ReadCloser).Close()
+	_ = s.Out.(io.WriteCloser).Close()
 	_, _ = s.pagerProcess.Wait()
 	s.pagerProcess = nil
 }
@@ -430,3 +435,16 @@ func terminalSize(w io.Writer) (int, int, error) {
 	}
 	return 0, 0, fmt.Errorf("%v is not a file", w)
 }
+
+// pagerWriter implements a WriteCloser that wraps all EPIPE errors in an ErrClosedPagerPipe type.
+type pagerWriter struct {
+	io.WriteCloser
+}
+
+func (w *pagerWriter) Write(d []byte) (int, error) {
+	n, err := w.WriteCloser.Write(d)
+	if err != nil && (errors.Is(err, io.ErrClosedPipe) || isEpipeError(err)) {
+		return n, &ErrClosedPagerPipe{err}
+	}
+	return n, err
+}