plugin/log: remove ErrorFunc (coredns#2716)

The server handles this case no need to also do it in the log plugin. Means DefaultErrorFunc can be private to the dnsserver and is now renamed to just errorFunc Fixes: coredns#2715 Signed-off-by: Miek Gieben <miek@miek.nl>
horahoradev · Mar 25, 2019 · 45624a0 · 45624a0
1 parent f08f7e2
commit 45624a0
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 34 deletions.
diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go
@@ -205,7 +205,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	// The default dns.Mux checks the question section size, but we have our
 	// own mux here. Check if we have a question section. If not drop them here.
 	if r == nil || len(r.Question) == 0 {
-		DefaultErrorFunc(ctx, w, r, dns.RcodeServerFailure)
+		errorFunc(ctx, w, r, dns.RcodeServerFailure)
 		return
 	}
 
@@ -215,13 +215,13 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 			// need to make sure that we stay alive up here
 			if rec := recover(); rec != nil {
 				vars.Panic.Inc()
-				DefaultErrorFunc(ctx, w, r, dns.RcodeServerFailure)
+				errorFunc(ctx, w, r, dns.RcodeServerFailure)
 			}
 		}()
 	}
 
 	if !s.classChaos && r.Question[0].Qclass != dns.ClassINET {
-		DefaultErrorFunc(ctx, w, r, dns.RcodeRefused)
+		errorFunc(ctx, w, r, dns.RcodeRefused)
 		return
 	}
 
@@ -260,7 +260,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 				if h.FilterFunc == nil {
 					rcode, _ := h.pluginChain.ServeDNS(ctx, w, r)
 					if !plugin.ClientWrite(rcode) {
-						DefaultErrorFunc(ctx, w, r, rcode)
+						errorFunc(ctx, w, r, rcode)
 					}
 					return
 				}
@@ -269,7 +269,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 				if h.FilterFunc(q) {
 					rcode, _ := h.pluginChain.ServeDNS(ctx, w, r)
 					if !plugin.ClientWrite(rcode) {
-						DefaultErrorFunc(ctx, w, r, rcode)
+						errorFunc(ctx, w, r, rcode)
 					}
 					return
 				}
@@ -291,7 +291,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		// DS request, and we found a zone, use the handler for the query.
 		rcode, _ := dshandler.pluginChain.ServeDNS(ctx, w, r)
 		if !plugin.ClientWrite(rcode) {
-			DefaultErrorFunc(ctx, w, r, rcode)
+			errorFunc(ctx, w, r, rcode)
 		}
 		return
 	}
@@ -304,13 +304,13 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 
 		rcode, _ := h.pluginChain.ServeDNS(ctx, w, r)
 		if !plugin.ClientWrite(rcode) {
-			DefaultErrorFunc(ctx, w, r, rcode)
+			errorFunc(ctx, w, r, rcode)
 		}
 		return
 	}
 
 	// Still here? Error out with REFUSED.
-	DefaultErrorFunc(ctx, w, r, dns.RcodeRefused)
+	errorFunc(ctx, w, r, dns.RcodeRefused)
 }
 
 // OnStartupComplete lists the sites served by this server
@@ -336,8 +336,8 @@ func (s *Server) Tracer() ot.Tracer {
 	return s.trace.Tracer()
 }
 
-// DefaultErrorFunc responds to an DNS request with an error.
-func DefaultErrorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) {
+// errorFunc responds to an DNS request with an error.
+func errorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) {
 	state := request.Request{W: w, Req: r}
 
 	answer := new(dns.Msg)

diff --git a/plugin/log/log.go b/plugin/log/log.go
@@ -6,10 +6,8 @@ import (
 	"time"
 
 	"github.com/coredns/coredns/plugin"
-	"github.com/coredns/coredns/plugin/metrics/vars"
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	clog "github.com/coredns/coredns/plugin/pkg/log"
-	"github.com/coredns/coredns/plugin/pkg/rcode"
 	"github.com/coredns/coredns/plugin/pkg/replacer"
 	"github.com/coredns/coredns/plugin/pkg/response"
 	"github.com/coredns/coredns/request"
@@ -19,9 +17,8 @@ import (
 
 // Logger is a basic request logging plugin.
 type Logger struct {
-	Next      plugin.Handler
-	Rules     []Rule
-	ErrorFunc func(context.Context, dns.ResponseWriter, *dns.Msg, int) // failover error handler
+	Next  plugin.Handler
+	Rules []Rule
 
 	repl replacer.Replacer
 }
@@ -37,22 +34,6 @@ func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		rrw := dnstest.NewRecorder(w)
 		rc, err := plugin.NextOrFailure(l.Name(), l.Next, ctx, rrw, r)
 
-		if rc > 0 {
-			// 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(ctx, rrw, r, rc)
-			} else {
-				answer := new(dns.Msg)
-				answer.SetRcode(r, rc)
-
-				vars.Report(ctx, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now())
-
-				w.WriteMsg(answer)
-			}
-			rc = 0
-		}
-
 		tpe, _ := response.Typify(rrw.Msg, time.Now().UTC())
 		class := response.Classify(tpe)
 		// If we don't set up a class in config, the default "all" will be added

diff --git a/plugin/log/log_test.go b/plugin/log/log_test.go
@@ -41,8 +41,8 @@ func TestLoggedStatus(t *testing.T) {
 	rec := dnstest.NewRecorder(&test.ResponseWriter{})
 
 	rcode, _ := logger.ServeDNS(ctx, rec, r)
-	if rcode != 0 {
-		t.Errorf("Expected rcode to be 0 - was: %d", rcode)
+	if rcode != 2 {
+		t.Errorf("Expected rcode to be 2 - was: %d", rcode)
 	}
 
 	logged := f.String()

diff --git a/plugin/log/setup.go b/plugin/log/setup.go
@@ -26,7 +26,7 @@ func setup(c *caddy.Controller) error {
 	}
 
 	dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
-		return Logger{Next: next, Rules: rules, ErrorFunc: dnsserver.DefaultErrorFunc, repl: replacer.New()}
+		return Logger{Next: next, Rules: rules, repl: replacer.New()}
 	})
 
 	return nil