godoc: set expvar for http.ResponseWriter errors.

R=bradfitz
CC=adg, golang-codereviews
https://golang.org/cl/56680044

godoc/page.go

@@ -5,7 +5,6 @@
 package godoc
 
 import (
-	"log"
 	"net/http"
 	"runtime"
 )
@@ -31,11 +30,7 @@ func (p *Presentation) ServePage(w http.ResponseWriter, page Page) {
 	page.SearchBox = p.Corpus.IndexEnabled
 	page.Playground = p.ShowPlayground
 	page.Version = runtime.Version()
-	if err := p.GodocHTML.Execute(w, page); err != nil && err != http.ErrBodyNotAllowed {
+	applyTemplateToResponseWriter(w, p.GodocHTML, page)
-		// Only log if there's an error that's not about writing on HEAD requests.
-		// See Issues 5451 and 5454.
-		log.Printf("GodocHTML.Execute: %s", err)
-	}
 }
 
 func (p *Presentation) ServeError(w http.ResponseWriter, r *http.Request, relpath string, err error) {

godoc/search.go

@@ -7,7 +7,6 @@ package godoc
 import (
 	"bytes"
 	"fmt"
-	"log"
 	"net/http"
 	"regexp"
 	"strings"
@@ -135,9 +134,5 @@ func (p *Presentation) serveSearchDesc(w http.ResponseWriter, r *http.Request) {
 	data := map[string]interface{}{
 		"BaseURL": fmt.Sprintf("http://%s", r.Host),
 	}
-	if err := p.SearchDescXML.Execute(w, &data); err != nil && err != http.ErrBodyNotAllowed {
+	applyTemplateToResponseWriter(w, p.SearchDescXML, &data)
-		// Only log if there's an error that's not about writing on HEAD requests.
-		// See Issues 5451 and 5454.
-		log.Printf("searchDescXML.Execute: %s", err)
-	}
 }

godoc/server.go

@@ -6,6 +6,7 @@ package godoc
 
 import (
 	"bytes"
+	"expvar"
 	"fmt"
 	"go/ast"
 	"go/build"
@@ -392,6 +393,45 @@ func applyTemplate(t *template.Template, name string, data interface{}) []byte {
 	return buf.Bytes()
 }
 
+type writerCapturesErr struct {
+	w   io.Writer
+	err error
+}
+
+func (w *writerCapturesErr) Write(p []byte) (int, error) {
+	n, err := w.w.Write(p)
+	if err != nil {
+		w.err = err
+	}
+	return n, err
+}
+
+var httpErrors *expvar.Map
+
+func init() {
+	httpErrors = expvar.NewMap("httpWriteErrors").Init()
+}
+
+// applyTemplateToResponseWriter uses an http.ResponseWriter as the io.Writer
+// for the call to template.Execute.  It uses an io.Writer wrapper to capture
+// errors from the underlying http.ResponseWriter.  If an error is found, an
+// expvar will be incremented.  Other template errors will be logged.  This is
+// done to keep from polluting log files with error messages due to networking
+// issues, such as client disconnects and http HEAD protocol violations.
+func applyTemplateToResponseWriter(rw http.ResponseWriter, t *template.Template, data interface{}) {
+	w := &writerCapturesErr{w: rw}
+	err := t.Execute(w, data)
+	// There are some cases where template.Execute does not return an error when
+	// rw returns an error, and some where it does.  So check w.err first.
+	if w.err != nil {
+		// For http errors, increment an expvar.
+		httpErrors.Add(w.err.Error(), 1)
+	} else if err != nil {
+		// Log template errors.
+		log.Printf("%s.Execute: %s", t.Name(), err)
+	}
+}
+
 func redirect(w http.ResponseWriter, r *http.Request) (redirected bool) {
 	canonical := pathpkg.Clean(r.URL.Path)
 	if !strings.HasSuffix(canonical, "/") {

godoc/server_test.go

@@ -0,0 +1,75 @@
+package godoc
+
+import (
+	"errors"
+	"expvar"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"text/template"
+)
+
+var (
+	// NOTE: with no plain-text in the template, template.Execute will not
+	// return an error when http.ResponseWriter.Write does return an error.
+	tmpl = template.Must(template.New("test").Parse("{{.Foo}}"))
+)
+
+type withFoo struct {
+	Foo int
+}
+
+type withoutFoo struct {
+}
+
+type errResponseWriter struct {
+}
+
+func (*errResponseWriter) Header() http.Header {
+	return http.Header{}
+}
+
+func (*errResponseWriter) WriteHeader(int) {
+}
+
+func (*errResponseWriter) Write(p []byte) (int, error) {
+	return 0, errors.New("error")
+}
+
+func TestApplyTemplateToResponseWriter(t *testing.T) {
+	for _, tc := range []struct {
+		desc    string
+		rw      http.ResponseWriter
+		data    interface{}
+		expVars int
+	}{
+		{
+			desc:    "no error",
+			rw:      &httptest.ResponseRecorder{},
+			data:    &withFoo{},
+			expVars: 0,
+		},
+		{
+			desc:    "template error",
+			rw:      &httptest.ResponseRecorder{},
+			data:    &withoutFoo{},
+			expVars: 0,
+		},
+		{
+			desc:    "ResponseWriter error",
+			rw:      &errResponseWriter{},
+			data:    &withFoo{},
+			expVars: 1,
+		},
+	} {
+		httpErrors.Init()
+		applyTemplateToResponseWriter(tc.rw, tmpl, tc.data)
+		gotVars := 0
+		httpErrors.Do(func(expvar.KeyValue) {
+			gotVars++
+		})
+		if gotVars != tc.expVars {
+			t.Errorf("applyTemplateToResponseWriter(%q): got %d vars, want %d", tc.desc, gotVars, tc.expVars)
+		}
+	}
+}