plugin/log: remove ErrorFunc (#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: #2715

Signed-off-by: Miek Gieben <miek@miek.nl>

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.
+// errorFunc responds to an DNS request with an error.
-func DefaultErrorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) {
+func errorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) {
 	state := request.Request{W: w, Req: r}
 
 	answer := new(dns.Msg)

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"
@@ -21,7 +19,6 @@ import (
 type Logger struct {
 	Next  plugin.Handler
 	Rules []Rule
-	ErrorFunc func(context.Context, dns.ResponseWriter, *dns.Msg, int) // failover error handler
 
 	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

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 {
+	if rcode != 2 {
-		t.Errorf("Expected rcode to be 0 - was: %d", rcode)
+		t.Errorf("Expected rcode to be 2 - was: %d", rcode)
 	}
 
 	logged := f.String()

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