Implement suberrors & subproblems (#4227)

Updates #4193

Updating relevant Boulder locations to use WithSubErrors and WithSubProblems will be done in a separate follow-up PR.

errors/errors.go

@@ -1,6 +1,10 @@
 package errors
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/letsencrypt/boulder/identifier"
+)
 
 // ErrorType provides a coarse category for BoulderErrors
 type ErrorType int
@@ -24,14 +28,32 @@ const (
 
 // BoulderError represents internal Boulder errors
 type BoulderError struct {
-	Type   ErrorType
-	Detail string
+	Type      ErrorType
+	Detail    string
+	SubErrors []SubBoulderError
+}
+
+// SubBoulderError represents sub-errors specific to an identifier that are
+// related to a top-level internal Boulder error.
+type SubBoulderError struct {
+	*BoulderError
+	Identifier identifier.ACMEIdentifier
 }
 
 func (be *BoulderError) Error() string {
 	return be.Detail
 }
 
+// WithSubErrors returns a new BoulderError instance created by adding the
+// provided subErrs to the existing BoulderError.
+func (be *BoulderError) WithSubErrors(subErrs []SubBoulderError) *BoulderError {
+	return &BoulderError{
+		Type:      be.Type,
+		Detail:    be.Detail,
+		SubErrors: append(be.SubErrors, subErrs...),
+	}
+}
+
 // New is a convenience function for creating a new BoulderError
 func New(errType ErrorType, msg string, args ...interface{}) error {
 	return &BoulderError{

errors/errors_test.go

@@ -0,0 +1,50 @@
+package errors
+
+import (
+	"testing"
+
+	"github.com/letsencrypt/boulder/identifier"
+	"github.com/letsencrypt/boulder/test"
+)
+
+// TestWithSubErrors tests that a boulder error can be created by adding
+// suberrors to an existing top level boulder error
+func TestWithSubErrors(t *testing.T) {
+	topErr := &BoulderError{
+		Type:   RateLimit,
+		Detail: "don't you think you have enough certificates already?",
+	}
+
+	subErrs := []SubBoulderError{
+		SubBoulderError{
+			Identifier: identifier.DNSIdentifier("example.com"),
+			BoulderError: &BoulderError{
+				Type:   RateLimit,
+				Detail: "everyone uses this example domain",
+			},
+		},
+		SubBoulderError{
+			Identifier: identifier.DNSIdentifier("what about example.com"),
+			BoulderError: &BoulderError{
+				Type:   RateLimit,
+				Detail: "try a real identifier value next time",
+			},
+		},
+	}
+
+	outResult := topErr.WithSubErrors(subErrs)
+	// The outResult should be a new, distinct error
+	test.AssertNotEquals(t, topErr, outResult)
+	// The outResult error should have the correct sub errors
+	test.AssertDeepEquals(t, outResult.SubErrors, subErrs)
+	// Adding another suberr shouldn't squash the original sub errors
+	anotherSubErr := SubBoulderError{
+		Identifier: identifier.DNSIdentifier("another ident"),
+		BoulderError: &BoulderError{
+			Type:   RateLimit,
+			Detail: "another rate limit err",
+		},
+	}
+	outResult = outResult.WithSubErrors([]SubBoulderError{anotherSubErr})
+	test.AssertDeepEquals(t, outResult.SubErrors, append(subErrs, anotherSubErr))
+}

probs/probs.go

@@ -3,6 +3,8 @@ package probs
 import (
 	"fmt"
 	"net/http"
+
+	"github.com/letsencrypt/boulder/identifier"
 )
 
 // Error types that can be used in ACME payloads
@@ -40,12 +42,34 @@ type ProblemDetails struct {
 	// HTTPStatus is the HTTP status code the ProblemDetails should probably be sent
 	// as.
 	HTTPStatus int `json:"status,omitempty"`
+	// SubProblems are optional additional per-identifier problems. See
+	// RFC 8555 Section 6.7.1: https://tools.ietf.org/html/rfc8555#section-6.7.1
+	SubProblems []SubProblemDetails `json:"subproblems,omitempty"`
+}
+
+// SubProblemDetails represents sub-problems specific to an identifier that are
+// related to a top-level ProblemDetails.
+// See RFC 8555 Section 6.7.1: https://tools.ietf.org/html/rfc8555#section-6.7.1
+type SubProblemDetails struct {
+	ProblemDetails
+	Identifier identifier.ACMEIdentifier
 }
 
 func (pd *ProblemDetails) Error() string {
 	return fmt.Sprintf("%s :: %s", pd.Type, pd.Detail)
 }
 
+// WithSubProblems returns a new ProblemsDetails instance created by adding the
+// provided subProbs to the existing ProblemsDetail.
+func (pd *ProblemDetails) WithSubProblems(subProbs []SubProblemDetails) *ProblemDetails {
+	return &ProblemDetails{
+		Type:        pd.Type,
+		Detail:      pd.Detail,
+		HTTPStatus:  pd.HTTPStatus,
+		SubProblems: append(pd.SubProblems, subProbs...),
+	}
+}
+
 // statusTooManyRequests is the HTTP status code meant for rate limiting
 // errors. It's not currently in the net/http library so we add it here.
 const statusTooManyRequests = 429

probs/probs_test.go

@@ -5,6 +5,7 @@ import (
 
 	"net/http"
 
+	"github.com/letsencrypt/boulder/identifier"
 	"github.com/letsencrypt/boulder/test"
 )
 
@@ -77,5 +78,55 @@ func TestProblemDetailsConvenience(t *testing.T) {
 		if c.pb.Detail != c.detail {
 			t.Errorf("Incorrect detail message. Expected %s got %s", c.detail, c.pb.Detail)
 		}
+
+		if subProbLen := len(c.pb.SubProblems); subProbLen != 0 {
+			t.Errorf("Incorrect SubProblems. Expected 0, found %d", subProbLen)
+		}
 	}
 }
+
+// TestWithSubProblems tests that a new problem can be constructed by adding
+// subproblems.
+func TestWithSubProblems(t *testing.T) {
+	topProb := &ProblemDetails{
+		Type:       RateLimitedProblem,
+		Detail:     "don't you think you have enough certificates already?",
+		HTTPStatus: statusTooManyRequests,
+	}
+	subProbs := []SubProblemDetails{
+		SubProblemDetails{
+			Identifier: identifier.DNSIdentifier("example.com"),
+			ProblemDetails: ProblemDetails{
+				Type:       RateLimitedProblem,
+				Detail:     "don't you think you have enough certificates already?",
+				HTTPStatus: statusTooManyRequests,
+			},
+		},
+		SubProblemDetails{
+			Identifier: identifier.DNSIdentifier("what about example.com"),
+			ProblemDetails: ProblemDetails{
+				Type:       MalformedProblem,
+				Detail:     "try a real identifier value next time",
+				HTTPStatus: http.StatusConflict,
+			},
+		},
+	}
+
+	outResult := topProb.WithSubProblems(subProbs)
+
+	// The outResult should be a new, distinct problem details instance
+	test.AssertNotEquals(t, topProb, outResult)
+	// The outResult problem details should have the correct sub problems
+	test.AssertDeepEquals(t, outResult.SubProblems, subProbs)
+	// Adding another sub problem shouldn't squash the original sub problems
+	anotherSubProb := SubProblemDetails{
+		Identifier: identifier.DNSIdentifier("another ident"),
+		ProblemDetails: ProblemDetails{
+			Type:       RateLimitedProblem,
+			Detail:     "yet another rate limit err",
+			HTTPStatus: statusTooManyRequests,
+		},
+	}
+	outResult = outResult.WithSubProblems([]SubProblemDetails{anotherSubProb})
+	test.AssertDeepEquals(t, outResult.SubProblems, append(subProbs, anotherSubProb))
+}