Fix error reporting (#128)

Put error back in the correct place in the directives.go. Also don't
make it a pointer. If it *is* a pointer the buildstack function does
not correctly set the Next Handler. Don't understand *why* this is
different from Caddy. Anyway this fixes it, with the caveat that
the error log file is now openend earlier in the startup.

Fixes #127

core/directives.go

@@ -53,7 +53,9 @@ var directiveOrder = []directive{
 
 	// Directives that inject handlers (middleware)
 	{"prometheus", setup.Prometheus},
+	{"errors", setup.Errors},
 	{"log", setup.Log},
+
 	{"chaos", setup.Chaos},
 	{"rewrite", setup.Rewrite},
 	{"loadbalance", setup.Loadbalance},
@@ -62,7 +64,6 @@ var directiveOrder = []directive{
 	{"secondary", setup.Secondary},
 	{"etcd", setup.Etcd},
 	{"proxy", setup.Proxy},
-	{"errors", setup.Errors},
 }
 
 // RegisterDirective adds the given directive to caddy's list of directives.
@@ -90,5 +91,5 @@ type directive struct {
 
 // SetupFunc takes a controller and may optionally return a middleware.
 // If the resulting middleware is not nil, it will be chained into
-// the HTTP handlers in the order specified in this package.
+// the DNS handlers in the order specified in this package.
 type SetupFunc func(c *setup.Controller) (middleware.Middleware, error)

core/setup/errors.go

@@ -5,9 +5,10 @@ import (
 	"log"
 	"os"
 
-	"github.com/hashicorp/go-syslog"
 	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/middleware/errors"
+
+	"github.com/hashicorp/go-syslog"
 )
 
 // Errors configures a new errors middleware instance.
@@ -17,9 +18,6 @@ func Errors(c *Controller) (middleware.Middleware, error) {
 		return nil, err
 	}
 
-	// Open the log file for writing when the server starts
-	c.Startup = append(c.Startup, func() error {
-		var err error
 	var writer io.Writer
 
 	switch handler.LogFile {
@@ -32,7 +30,7 @@ func Errors(c *Controller) (middleware.Middleware, error) {
 	case "syslog":
 		writer, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "coredns")
 		if err != nil {
-				return err
+			return nil, err
 		}
 	default:
 		if handler.LogFile == "" {
@@ -43,7 +41,7 @@ func Errors(c *Controller) (middleware.Middleware, error) {
 		var file *os.File
 		file, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
 		if err != nil {
-				return err
+			return nil, err
 		}
 		if handler.LogRoller != nil {
 			file.Close()
@@ -55,10 +53,7 @@ func Errors(c *Controller) (middleware.Middleware, error) {
 			writer = file
 		}
 	}
-
 	handler.Log = log.New(writer, "", 0)
-		return nil
-	})
 
 	return func(next middleware.Handler) middleware.Handler {
 		handler.Next = next
@@ -66,11 +61,8 @@ func Errors(c *Controller) (middleware.Middleware, error) {
 	}, nil
 }
 
-func errorsParse(c *Controller) (*errors.ErrorHandler, error) {
-	// Very important that we make a pointer because the Startup
-	// function that opens the log file must have access to the
-	// same instance of the handler, not a copy.
-	handler := &errors.ErrorHandler{}
+func errorsParse(c *Controller) (errors.ErrorHandler, error) {
+	handler := errors.ErrorHandler{}
 
 	optionalBlock := func() (bool, error) {
 		var hadBlock bool

middleware/etcd/handler.go

@@ -73,7 +73,6 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 		return e.Err(zone, dns.RcodeNameError, state)
 	}
 	if err != nil {
-		println("returning error", err.Error())
 		return dns.RcodeServerFailure, err
 	}