Use MatchWithError everywhere possible

5 files changed, 67 insertions, 39 deletions

diff --git a/modules/caddyhttp/celmatcher_test.go b/modules/caddyhttp/celmatcher_test.go
index 26491b7ca..a7e91529c 100644
--- a/modules/caddyhttp/celmatcher_test.go
+++ b/modules/caddyhttp/celmatcher_test.go
@@ -489,7 +489,11 @@ func TestMatchExpressionMatch(t *testing.T) {
 				}
 			}
 
-			if tc.expression.Match(req) != tc.wantResult {
+			matches, err := tc.expression.MatchWithError(req)
+			if err != nil {
+				t.Errorf("MatchExpression.Match() error = %v", err)
+			}
+			if matches != tc.wantResult {
 				t.Errorf("MatchExpression.Match() expected to return '%t', for expression : '%s'", tc.wantResult, tc.expression.Expr)
 			}
 		})
@@ -532,7 +536,7 @@ func BenchmarkMatchExpressionMatch(b *testing.B) {
 			}
 			b.ResetTimer()
 			for i := 0; i < b.N; i++ {
-				tc.expression.Match(req)
+				tc.expression.MatchWithError(req)
 			}
 		})
 	}
diff --git a/modules/caddyhttp/fileserver/matcher_test.go b/modules/caddyhttp/fileserver/matcher_test.go
index 95eeb8216..b6697b9d8 100644
--- a/modules/caddyhttp/fileserver/matcher_test.go
+++ b/modules/caddyhttp/fileserver/matcher_test.go
@@ -130,7 +130,10 @@ func TestFileMatcher(t *testing.T) {
 		req := &http.Request{URL: u}
 		repl := caddyhttp.NewTestReplacer(req)
 
-		result := m.Match(req)
+		result, err := m.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d: unexpected error: %v", i, err)
+		}
 		if result != tc.matched {
 			t.Errorf("Test %d: expected match=%t, got %t", i, tc.matched, result)
 		}
@@ -240,7 +243,10 @@ func TestPHPFileMatcher(t *testing.T) {
 		req := &http.Request{URL: u}
 		repl := caddyhttp.NewTestReplacer(req)
 
-		result := m.Match(req)
+		result, err := m.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d: unexpected error: %v", i, err)
+		}
 		if result != tc.matched {
 			t.Errorf("Test %d: expected match=%t, got %t", i, tc.matched, result)
 		}
@@ -389,7 +395,12 @@ func TestMatchExpressionMatch(t *testing.T) {
 			ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
 			req = req.WithContext(ctx)
 
-			if tc.expression.Match(req) != tc.wantResult {
+			matches, err := tc.expression.MatchWithError(req)
+			if err != nil {
+				t.Errorf("MatchExpression.Match() error = %v", err)
+				return
+			}
+			if matches != tc.wantResult {
 				t.Errorf("MatchExpression.Match() expected to return '%t', for expression : '%s'", tc.wantResult, tc.expression.Expr)
 			}
 
diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go
index 05eaade5b..f7be6909e 100644
--- a/modules/caddyhttp/matchers_test.go
+++ b/modules/caddyhttp/matchers_test.go
@@ -158,7 +158,10 @@ func TestHostMatcher(t *testing.T) {
 			t.Errorf("Test %d %v: provisioning failed: %v", i, tc.match, err)
 		}
 
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d %v: Expected %t, got %t for '%s'", i, tc.match, tc.expect, actual, tc.input)
 			continue
@@ -430,7 +433,10 @@ func TestPathMatcher(t *testing.T) {
 		ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
 		req = req.WithContext(ctx)
 
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d %v: Expected %t, got %t for '%s'", i, tc.match, tc.expect, actual, tc.input)
 			continue
@@ -451,7 +457,10 @@ func TestPathMatcherWindows(t *testing.T) {
 	req = req.WithContext(ctx)
 
 	match := MatchPath{"*.php"}
-	matched := match.Match(req)
+	matched, err := match.MatchWithError(req)
+	if err != nil {
+		t.Errorf("Expected no error, but got: %v", err)
+	}
 	if !matched {
 		t.Errorf("Expected to match; should ignore trailing dots and spaces")
 	}
@@ -555,7 +564,10 @@ func TestPathREMatcher(t *testing.T) {
 		req = req.WithContext(ctx)
 		addHTTPVarsToReplacer(repl, req, httptest.NewRecorder())
 
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d [%v]: Expected %t, got %t for input '%s'",
 				i, tc.match.Pattern, tc.expect, actual, tc.input)
@@ -691,7 +703,10 @@ func TestHeaderMatcher(t *testing.T) {
 		ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
 		req = req.WithContext(ctx)
 
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d %v: Expected %t, got %t for '%s'", i, tc.match, tc.expect, actual, tc.input)
 			continue
@@ -818,7 +833,10 @@ func TestQueryMatcher(t *testing.T) {
 		repl.Set("http.vars.debug", "1")
 		repl.Set("http.vars.key", "somekey")
 		req = req.WithContext(ctx)
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d %v: Expected %t, got %t for '%s'", i, tc.match, tc.expect, actual, tc.input)
 			continue
@@ -887,7 +905,10 @@ func TestHeaderREMatcher(t *testing.T) {
 		req = req.WithContext(ctx)
 		addHTTPVarsToReplacer(repl, req, httptest.NewRecorder())
 
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d [%v]: Expected %t, got %t for input '%s'",
 				i, tc.match, tc.expect, actual, tc.input)
@@ -927,7 +948,7 @@ func BenchmarkHeaderREMatcher(b *testing.B) {
 	req = req.WithContext(ctx)
 	addHTTPVarsToReplacer(repl, req, httptest.NewRecorder())
 	for run := 0; run < b.N; run++ {
-		match.Match(req)
+		match.MatchWithError(req)
 	}
 }
 
@@ -998,7 +1019,10 @@ func TestVarREMatcher(t *testing.T) {
 
 			tc.input.ServeHTTP(httptest.NewRecorder(), req, emptyHandler)
 
-			actual := tc.match.Match(req)
+			actual, err := tc.match.MatchWithError(req)
+			if err != nil {
+				t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+			}
 			if actual != tc.expect {
 				t.Errorf("Test %d [%v]: Expected %t, got %t for input '%s'",
 					i, tc.match, tc.expect, actual, tc.input)
@@ -1123,7 +1147,10 @@ func TestNotMatcher(t *testing.T) {
 		ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
 		req = req.WithContext(ctx)
 
-		actual := tc.match.Match(req)
+		actual, err := tc.match.MatchWithError(req)
+		if err != nil {
+			t.Errorf("Test %d %v: matching failed: %v", i, tc.match, err)
+		}
 		if actual != tc.expect {
 			t.Errorf("Test %d %+v: Expected %t, got %t for: host=%s path=%s'", i, tc.match, tc.expect, actual, tc.host, tc.path)
 			continue
@@ -1155,7 +1182,7 @@ func BenchmarkLargeHostMatcher(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		matcher.Match(req)
+		matcher.MatchWithError(req)
 	}
 }
 
@@ -1169,7 +1196,7 @@ func BenchmarkHostMatcherWithoutPlaceholder(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		match.Match(req)
+		match.MatchWithError(req)
 	}
 }
 
@@ -1187,6 +1214,6 @@ func BenchmarkHostMatcherWithPlaceholder(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		match.Match(req)
+		match.MatchWithError(req)
 	}
 }
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index 5ef37a4e8..f0e2e6ba1 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -1124,7 +1124,7 @@ func (lb LoadBalancing) tryAgain(ctx caddy.Context, start time.Time, retries int
 				return false
 			}
 
-			match, err := lb.RetryMatch.AnyMatchWithError(req)
+			match, err := lb.RetryMatch.AnyMatch(req)
 			if err != nil {
 				logger.Error("error matching request for retry", zap.Error(err))
 				return false
diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go
index 1e721229d..9576070f6 100644
--- a/modules/caddyhttp/routes.go
+++ b/modules/caddyhttp/routes.go
@@ -254,7 +254,7 @@ func wrapRoute(route Route) Middleware {
 			nextCopy := next
 
 			// route must match at least one of the matcher sets
-			matches, err := route.MatcherSets.AnyMatchWithError(req)
+			matches, err := route.MatcherSets.AnyMatch(req)
 			if err != nil {
 				// allow matchers the opportunity to short circuit
 				// the request and trigger the error handling chain
@@ -382,25 +382,11 @@ type RawMatcherSets []caddy.ModuleMap
 // the sets.
 type MatcherSets []MatcherSet
 
-// AnyMatch returns true if req matches any of the
-// matcher sets in ms or if there are no matchers,
-// in which case the request always matches.
-//
-// Deprecated: Use AnyMatchWithError instead.
-func (ms MatcherSets) AnyMatch(req *http.Request) bool {
-	for _, m := range ms {
-		if m.Match(req) {
-			return true
-		}
-	}
-	return len(ms) == 0
-}
-
-// AnyMatchWithError returns true if req matches any of the
-// matcher sets in ms or if there are no matchers, in which
-// case the request always matches. If any matcher returns
-// an error, we cut short and return the error.
-func (ms MatcherSets) AnyMatchWithError(req *http.Request) (bool, error) {
+// AnyMatch returns true if req matches any of the matcher sets
+// in ms or if there are no matchers, in which case the request
+// always matches. If any matcher returns an error, we cut short
+// and return the error.
+func (ms MatcherSets) AnyMatch(req *http.Request) (bool, error) {
 	for _, m := range ms {
 		match, err := m.MatchWithError(req)
 		if err != nil || match {