Errors directive testing and fixing

Drop a few tests and make it work and compile.
Also add the documentation: errors.md

middleware/errors/errors.go

@@ -24,12 +24,13 @@ type ErrorHandler struct {
 }
 
 func (h ErrorHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
-	defer h.recovery(w, r)
+	defer h.recovery(ctx, w, r)
 
 	rcode, err := h.Next.ServeDNS(ctx, w, r)
 
 	if err != nil {
-		errMsg := fmt.Sprintf("%s [ERROR %d %s %s] %v", time.Now().Format(timeFormat), rcode, r.Question[0].Name, dns.Type(r.Question[0].Qclass), err)
+		state := middleware.State{W: w, Req: r}
+		errMsg := fmt.Sprintf("%s [ERROR %d %s %s] %v", time.Now().Format(timeFormat), rcode, state.Name(), state.Type(), err)
 
 		if h.Debug {
 			// Write error to response as a txt message instead of to log
@@ -45,7 +46,7 @@ func (h ErrorHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns
 	return rcode, err
 }
 
-func (h ErrorHandler) recovery(w dns.ResponseWriter, r *dns.Msg) {
+func (h ErrorHandler) recovery(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) {
 	rec := recover()
 	if rec == nil {
 		return

middleware/errors/errors.md

@@ -0,0 +1,50 @@
+# errors
+
+`errors` allows you to set custom error pages and enable error logging.
+By default, error responses (HTTP status >= 400) are not logged and the client receives a plaintext error message.
+Using an error log, the text of each error will be recorded so you can determine what is going wrong without exposing those details to the clients. With error pages, you can present custom error messages and instruct your visitor what to do.
+
+
+## Syntax
+
+~~~
+errors [logfile]
+~~~
+
+* `logfile` is the path to the error log file to create (or append to), relative to the current working directory. It can also be stdout or stderr to write to the console, syslog to write to the system log (except on Windows), or visible to write the error (including full stack trace, if applicable) to the response. Writing errors to the response is NOT advised except in local debug situations. Default is stderr.
+The above syntax will simply enable error reporting on the server. To specify custom error pages, open a block:
+
+~~~
+errors {
+    what where
+}
+~~~
+
+* `what` can only `log`.
+* `where` is the path to the log file (as described above) and you can enable rotation to manage the log files.
+
+## Examples
+
+Log errors into a file in the parent directory:
+
+~~~
+errors ../error.log
+~~~
+
+Make errors visible to the client (for debugging only):
+
+~~~
+errors visible
+~~~
+
+Maintain error log files automatically:
+
+~~~
+errors {
+    log error.log {
+        size 50 # Rotate after 50 MB
+        age  30 # Keep rotated files for 30 days
+        keep 5  # Keep at most 5 log files
+    }
+}
+~~~

middleware/errors/errors_test.go

@@ -1,89 +1,53 @@
 package errors
 
-/*
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"log"
+	"strings"
+	"testing"
+
+	"golang.org/x/net/context"
+
+	"github.com/miekg/coredns/middleware"
+	"github.com/miekg/dns"
+)
+
 func TestErrors(t *testing.T) {
-	// create a temporary page
-	path := filepath.Join(os.TempDir(), "errors_test.html")
-	f, err := os.Create(path)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.Remove(path)
-
-	const content = "This is a error page"
-	_, err = f.WriteString(content)
-	if err != nil {
-		t.Fatal(err)
-	}
-	f.Close()
-
 	buf := bytes.Buffer{}
-	em := ErrorHandler{
-		ErrorPages: map[int]string{
-			http.StatusNotFound:  path,
-			http.StatusForbidden: "not_exist_file",
-		},
-		Log: log.New(&buf, "", 0),
-	}
-	_, notExistErr := os.Open("not_exist_file")
+	em := ErrorHandler{Log: log.New(&buf, "", 0)}
 
 	testErr := errors.New("test error")
 	tests := []struct {
 		next         middleware.Handler
 		expectedCode int
-		expectedBody string
 		expectedLog  string
 		expectedErr  error
 	}{
 		{
-			next:         genErrorHandler(http.StatusOK, nil, "normal"),
-			expectedCode: http.StatusOK,
-			expectedBody: "normal",
+			next:         genErrorHandler(dns.RcodeSuccess, nil),
+			expectedCode: dns.RcodeSuccess,
 			expectedLog:  "",
 			expectedErr:  nil,
 		},
 		{
-			next:         genErrorHandler(http.StatusMovedPermanently, testErr, ""),
-			expectedCode: http.StatusMovedPermanently,
-			expectedBody: "",
-			expectedLog:  fmt.Sprintf("[ERROR %d %s] %v\n", http.StatusMovedPermanently, "/", testErr),
+			next:         genErrorHandler(dns.RcodeNotAuth, testErr),
+			expectedCode: dns.RcodeNotAuth,
+			expectedLog:  fmt.Sprintf("[ERROR %d %s] %v\n", dns.RcodeNotAuth, "example.org. A", testErr),
 			expectedErr:  testErr,
 		},
-		{
-			next:         genErrorHandler(http.StatusBadRequest, nil, ""),
-			expectedCode: 0,
-			expectedBody: fmt.Sprintf("%d %s\n", http.StatusBadRequest,
-				http.StatusText(http.StatusBadRequest)),
-			expectedLog: "",
-			expectedErr: nil,
-		},
-		{
-			next:         genErrorHandler(http.StatusNotFound, nil, ""),
-			expectedCode: 0,
-			expectedBody: content,
-			expectedLog:  "",
-			expectedErr:  nil,
-		},
-		{
-			next:         genErrorHandler(http.StatusForbidden, nil, ""),
-			expectedCode: 0,
-			expectedBody: fmt.Sprintf("%d %s\n", http.StatusForbidden,
-				http.StatusText(http.StatusForbidden)),
-			expectedLog: fmt.Sprintf("[NOTICE %d /] could not load error page: %v\n",
-				http.StatusForbidden, notExistErr),
-			expectedErr: nil,
-		},
 	}
 
-	req, err := http.NewRequest("GET", "/", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	ctx := context.TODO()
+	req := new(dns.Msg)
+	req.SetQuestion("example.org.", dns.TypeA)
+
 	for i, test := range tests {
 		em.Next = test.next
 		buf.Reset()
-		rec := httptest.NewRecorder()
-		code, err := em.ServeHTTP(rec, req)
+		rec := middleware.NewResponseRecorder(&middleware.TestResponseWriter{})
+		code, err := em.ServeDNS(ctx, rec, req)
 
 		if err != test.expectedErr {
 			t.Errorf("Test %d: Expected error %v, but got %v",
@@ -93,10 +57,6 @@ func TestErrors(t *testing.T) {
 			t.Errorf("Test %d: Expected status code %d, but got %d",
 				i, test.expectedCode, code)
 		}
-		if body := rec.Body.String(); body != test.expectedBody {
-			t.Errorf("Test %d: Expected body %q, but got %q",
-				i, test.expectedBody, body)
-		}
 		if log := buf.String(); !strings.Contains(log, test.expectedLog) {
 			t.Errorf("Test %d: Expected log %q, but got %q",
 				i, test.expectedLog, log)
@@ -107,48 +67,29 @@ func TestErrors(t *testing.T) {
 func TestVisibleErrorWithPanic(t *testing.T) {
 	const panicMsg = "I'm a panic"
 	eh := ErrorHandler{
-		ErrorPages: make(map[int]string),
-		Debug:      true,
-		Next: middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
+		Debug: true,
+		Next: middleware.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 			panic(panicMsg)
 		}),
 	}
 
-	req, err := http.NewRequest("GET", "/", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	rec := httptest.NewRecorder()
+	ctx := context.TODO()
+	req := new(dns.Msg)
+	req.SetQuestion("example.org.", dns.TypeA)
 
-	code, err := eh.ServeHTTP(rec, req)
+	rec := middleware.NewResponseRecorder(&middleware.TestResponseWriter{})
 
+	code, err := eh.ServeDNS(ctx, rec, req)
 	if code != 0 {
 		t.Errorf("Expected error handler to return 0 (it should write to response), got status %d", code)
 	}
 	if err != nil {
 		t.Errorf("Expected error handler to return nil error (it should panic!), but got '%v'", err)
 	}
-
-	body := rec.Body.String()
-
-	if !strings.Contains(body, "[PANIC /] middleware/errors/errors_test.go") {
-		t.Errorf("Expected response body to contain error log line, but it didn't:\n%s", body)
-	}
-	if !strings.Contains(body, panicMsg) {
-		t.Errorf("Expected response body to contain panic message, but it didn't:\n%s", body)
-	}
-	if len(body) < 500 {
-		t.Errorf("Expected response body to contain stack trace, but it was too short: len=%d", len(body))
-	}
 }
 
-func genErrorHandler(status int, err error, body string) middleware.Handler {
-	return middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
-		if len(body) > 0 {
-			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
-			fmt.Fprint(w, body)
-		}
-		return status, err
+func genErrorHandler(rcode int, err error) middleware.Handler {
+	return middleware.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+		return rcode, err
 	})
 }
-*/

middleware/log/log.md

@@ -1,6 +1,6 @@
-# Log
+# log
 
-log enables request logging. The request log is also known from some vernaculars as an access log.
+`log` enables request logging. The request log is also known from some vernaculars as an access log.
 
 ## Syntax