Skip to content

Commit

Permalink
fix: prevent side-effects of additional retry-able errors
Browse files Browse the repository at this point in the history
This commit removes `IdempotentRequestEOF` and `IncompleteRequest` from
the fail-able and prune-able classifier group. This prevents errors that
should not affect the endpoint from being marked as failed or being
pruned. To do so all classifiers groups are split into distinct groups
and any cross references between them are removed.

The main motivation for this change is to avoid confusion and bugs due to
artificial dependencies between the groups.

Partially-Resolves: cloudfoundry/routing-release#321
Tested-By: `routing-release/scripts/run-unit-tests-in-docker gorouter`
  • Loading branch information
maxmoehl authored and geofffranks committed Jul 27, 2023
1 parent 7c163e5 commit b820e88
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
20 changes: 18 additions & 2 deletions proxy/fails/classifier_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,27 @@ var RetriableClassifiers = ClassifierGroup{
}

var FailableClassifiers = ClassifierGroup{
RetriableClassifiers,
Dial,
AttemptedTLSWithNonTLSBackend,
HostnameMismatch,
RemoteFailedCertCheck,
RemoteHandshakeFailure,
RemoteHandshakeTimeout,
UntrustedCert,
ExpiredOrNotYetValidCertFailure,
ConnectionResetOnRead,
}

var PrunableClassifiers = RetriableClassifiers
var PrunableClassifiers = ClassifierGroup{
Dial,
AttemptedTLSWithNonTLSBackend,
HostnameMismatch,
RemoteFailedCertCheck,
RemoteHandshakeFailure,
RemoteHandshakeTimeout,
UntrustedCert,
ExpiredOrNotYetValidCertFailure,
}

// Classify returns true on errors that are retryable
func (cg ClassifierGroup) Classify(err error) bool {
Expand Down
4 changes: 1 addition & 3 deletions proxy/fails/classifier_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ var _ = Describe("ClassifierGroup", func() {
})

Describe("prunable", func() {
It("matches hostname mismatch", func() {
It("matches prunable errors", func() {
pc := fails.PrunableClassifiers

Expect(pc.Classify(&net.OpError{Op: "dial"})).To(BeTrue())
Expand All @@ -69,8 +69,6 @@ var _ = Describe("ClassifierGroup", func() {
Expect(pc.Classify(x509.UnknownAuthorityError{})).To(BeTrue())
Expect(pc.Classify(x509.CertificateInvalidError{Reason: x509.Expired})).To(BeTrue())
Expect(pc.Classify(errors.New("i'm a potato"))).To(BeFalse())
Expect(pc.Classify(fails.IdempotentRequestEOFError)).To(BeTrue())
Expect(pc.Classify(fails.IncompleteRequestError)).To(BeTrue())
})
})
})

0 comments on commit b820e88

Please sign in to comment.