From e726dca2eec90a01e26487aaaa4adfd27e364445 Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Tue, 19 Apr 2016 22:51:23 +0100
Subject: [PATCH] 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         |  5 ++-
 core/setup/errors.go       | 82 +++++++++++++++++---------------------
 middleware/etcd/handler.go |  1 -
 3 files changed, 40 insertions(+), 48 deletions(-)

diff --git a/core/directives.go b/core/directives.go
index 2529dacb1..c6cb81592 100644
--- a/core/directives.go
+++ b/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)
diff --git a/core/setup/errors.go b/core/setup/errors.go
index d99751bf4..bf6b56f87 100644
--- a/core/setup/errors.go
+++ b/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,48 +18,42 @@ 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
+	var writer io.Writer
 
-		switch handler.LogFile {
-		case "visible":
-			handler.Debug = true
-		case "stdout":
-			writer = os.Stdout
-		case "stderr":
-			writer = os.Stderr
-		case "syslog":
-			writer, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "coredns")
-			if err != nil {
-				return err
-			}
-		default:
-			if handler.LogFile == "" {
-				writer = os.Stderr // default
-				break
-			}
-
-			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
-			}
-			if handler.LogRoller != nil {
-				file.Close()
-
-				handler.LogRoller.Filename = handler.LogFile
-
-				writer = handler.LogRoller.GetLogWriter()
-			} else {
-				writer = file
-			}
+	switch handler.LogFile {
+	case "visible":
+		handler.Debug = true
+	case "stdout":
+		writer = os.Stdout
+	case "stderr":
+		writer = os.Stderr
+	case "syslog":
+		writer, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "coredns")
+		if err != nil {
+			return nil, err
+		}
+	default:
+		if handler.LogFile == "" {
+			writer = os.Stderr // default
+			break
 		}
 
-		handler.Log = log.New(writer, "", 0)
-		return nil
-	})
+		var file *os.File
+		file, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
+		if err != nil {
+			return nil, err
+		}
+		if handler.LogRoller != nil {
+			file.Close()
+
+			handler.LogRoller.Filename = handler.LogFile
+
+			writer = handler.LogRoller.GetLogWriter()
+		} else {
+			writer = file
+		}
+	}
+	handler.Log = log.New(writer, "", 0)
 
 	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
diff --git a/middleware/etcd/handler.go b/middleware/etcd/handler.go
index 729b049cc..38e1b51cd 100644
--- a/middleware/etcd/handler.go
+++ b/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
 	}