Merge branch 'master' of https://github.com/mholt/caddy

config/setup/log.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/mholt/caddy/middleware"
 	caddylog "github.com/mholt/caddy/middleware/log"
+	"github.com/mholt/caddy/server"
 )
 
 // Log sets up the logging middleware.
@@ -39,7 +40,7 @@ func Log(c *Controller) (middleware.Middleware, error) {
 	})
 
 	return func(next middleware.Handler) middleware.Handler {
-		return caddylog.Logger{Next: next, Rules: rules}
+		return caddylog.Logger{Next: next, Rules: rules, ErrorFunc: server.DefaultErrorFunc}
 	}, nil
 }
 

middleware/log/log.go

@@ -2,6 +2,7 @@
 package log
 
 import (
+	"fmt"
 	"log"
 	"net/http"
 
@@ -12,6 +13,7 @@ import (
 type Logger struct {
 	Next      middleware.Handler
 	Rules     []Rule
+	ErrorFunc func(http.ResponseWriter, *http.Request, int) // failover error handler
 }
 
 func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
@@ -19,6 +21,18 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 		if middleware.Path(r.URL.Path).Matches(rule.PathScope) {
 			responseRecorder := middleware.NewResponseRecorder(w)
 			status, err := l.Next.ServeHTTP(responseRecorder, r)
+			if status >= 400 {
+				// There was an error up the chain, but no response has been written yet.
+				// The error must be handled here so the log entry will record the response size.
+				if l.ErrorFunc != nil {
+					l.ErrorFunc(responseRecorder, r, status)
+				} else {
+					// Default failover error handler
+					responseRecorder.WriteHeader(status)
+					fmt.Fprintf(responseRecorder, "%d %s", status, http.StatusText(status))
+				}
+				status = 0
+			}
 			rep := middleware.NewReplacer(r, responseRecorder)
 			rule.Log.Println(rep.Replace(rule.Format))
 			return status, err

middleware/log/log_test.go

+package log
+
+import (
+	"bytes"
+	"log"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+type erroringMiddleware struct{}
+
+func (erroringMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
+	return http.StatusNotFound, nil
+}
+
+func TestLoggedStatus(t *testing.T) {
+	var f bytes.Buffer
+	var next erroringMiddleware
+	rule := Rule{
+		PathScope: "/",
+		Format:    DefaultLogFormat,
+		Log:       log.New(&f, "", 0),
+	}
+
+	logger := Logger{
+		Rules: []Rule{rule},
+		Next:  next,
+	}
+
+	r, err := http.NewRequest("GET", "/", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rec := httptest.NewRecorder()
+
+	status, err := logger.ServeHTTP(rec, r)
+	if status != 0 {
+		t.Error("Expected status to be 0 - was", status)
+	}
+
+	logged := f.String()
+	if !strings.Contains(logged, "404 13") {
+		t.Error("Expected 404 to be logged. Logged string -", logged)
+	}
+}

server/server.go

@@ -221,11 +221,15 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 		// Fallback error response in case error handling wasn't chained in
 		if status >= 400 {
-			w.WriteHeader(status)
-			fmt.Fprintf(w, "%d %s", status, http.StatusText(status))
+			DefaultErrorFunc(w, r, status)
 		}
 	} else {
 		w.WriteHeader(http.StatusNotFound)
 		fmt.Fprintf(w, "No such host at %s", s.address)
 	}
 }
+
+func DefaultErrorFunc(w http.ResponseWriter, r *http.Request, status int) {
+	w.WriteHeader(status)
+	fmt.Fprintf(w, "%d %s", status, http.StatusText(status))
+}