Skip to content

Commit 474e9d5

Browse files
committed
ProblemDetails no longer implements Error
1 parent 53c35ac commit 474e9d5

13 files changed

Lines changed: 90 additions & 92 deletions

File tree

errors/errors.go

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,23 @@
1-
// Package errors provides internal-facing error types for use in Boulder. Many
2-
// of these are transformed directly into Problem Details documents by the WFE.
3-
// Some, like NotFound, may be handled internally. We avoid using Problem
4-
// Details documents as part of our internal error system to avoid layering
5-
// confusions.
1+
// Package errors provide a special error type for use in Boulder. This error
2+
// type carries additional type information with it, and has two special powers:
63
//
7-
// These errors are specifically for use in errors that cross RPC boundaries.
8-
// An error type that does not need to be passed through an RPC can use a plain
9-
// Go type locally. Our gRPC code is aware of these error types and will
10-
// serialize and deserialize them automatically.
4+
// 1. It is recognized by our gRPC code, and the type metadata and detail string
5+
// will cross gRPC boundaries intact.
6+
//
7+
// 2. It is recognized by our frontend API "rendering" code, and will be
8+
// automatically converted to the corresponding urn:ietf:params:acme:error:...
9+
// ACME Problem Document.
10+
//
11+
// This means that a deeply-nested service (such as the SA) that wants to ensure
12+
// that the ACME client sees a particular problem document (such as NotFound)
13+
// can return a BoulderError and be sure that it will be propagated all the way
14+
// to the client.
15+
//
16+
// Note, however, that any additional context wrapped *around* the BoulderError
17+
// (such as by fmt.Errorf("oops: %w")) will be lost when the error is converted
18+
// into a problem document. Similarly, any type information wrapped *by* a
19+
// BoulderError (such as a sql.ErrNoRows) is lost at the gRPC serialization
20+
// boundary.
1121
package errors
1222

1323
import (
@@ -85,10 +95,15 @@ type SubBoulderError struct {
8595
Identifier identifier.ACMEIdentifier
8696
}
8797

98+
// Error implements the error interface, returning a string representation of
99+
// this error.
88100
func (be *BoulderError) Error() string {
89101
return be.Detail
90102
}
91103

104+
// Unwrap implements the optional error-unwrapping interface. It returns the
105+
// underlying type, all of when themselves implement the error interface, so
106+
// that `if errors.Is(someError, berrors.Malformed)` works.
92107
func (be *BoulderError) Unwrap() error {
93108
return be.Type
94109
}
@@ -164,134 +179,134 @@ func New(errType ErrorType, msg string) error {
164179

165180
// newf is a convenience function for creating a new BoulderError with a
166181
// formatted message.
167-
func newf(errType ErrorType, msg string, args ...interface{}) error {
182+
func newf(errType ErrorType, msg string, args ...any) error {
168183
return &BoulderError{
169184
Type: errType,
170185
Detail: fmt.Sprintf(msg, args...),
171186
}
172187
}
173188

174-
func InternalServerError(msg string, args ...interface{}) error {
189+
func InternalServerError(msg string, args ...any) error {
175190
return newf(InternalServer, msg, args...)
176191
}
177192

178-
func MalformedError(msg string, args ...interface{}) error {
193+
func MalformedError(msg string, args ...any) error {
179194
return newf(Malformed, msg, args...)
180195
}
181196

182-
func UnauthorizedError(msg string, args ...interface{}) error {
197+
func UnauthorizedError(msg string, args ...any) error {
183198
return newf(Unauthorized, msg, args...)
184199
}
185200

186-
func NotFoundError(msg string, args ...interface{}) error {
201+
func NotFoundError(msg string, args ...any) error {
187202
return newf(NotFound, msg, args...)
188203
}
189204

190-
func RateLimitError(retryAfter time.Duration, msg string, args ...interface{}) error {
205+
func RateLimitError(retryAfter time.Duration, msg string, args ...any) error {
191206
return &BoulderError{
192207
Type: RateLimit,
193208
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/", args...),
194209
RetryAfter: retryAfter,
195210
}
196211
}
197212

198-
func RegistrationsPerIPAddressError(retryAfter time.Duration, msg string, args ...interface{}) error {
213+
func RegistrationsPerIPAddressError(retryAfter time.Duration, msg string, args ...any) error {
199214
return &BoulderError{
200215
Type: RateLimit,
201216
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-registrations-per-ip-address", args...),
202217
RetryAfter: retryAfter,
203218
}
204219
}
205220

206-
func RegistrationsPerIPv6RangeError(retryAfter time.Duration, msg string, args ...interface{}) error {
221+
func RegistrationsPerIPv6RangeError(retryAfter time.Duration, msg string, args ...any) error {
207222
return &BoulderError{
208223
Type: RateLimit,
209224
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-registrations-per-ipv6-range", args...),
210225
RetryAfter: retryAfter,
211226
}
212227
}
213228

214-
func NewOrdersPerAccountError(retryAfter time.Duration, msg string, args ...interface{}) error {
229+
func NewOrdersPerAccountError(retryAfter time.Duration, msg string, args ...any) error {
215230
return &BoulderError{
216231
Type: RateLimit,
217232
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-orders-per-account", args...),
218233
RetryAfter: retryAfter,
219234
}
220235
}
221236

222-
func CertificatesPerDomainError(retryAfter time.Duration, msg string, args ...interface{}) error {
237+
func CertificatesPerDomainError(retryAfter time.Duration, msg string, args ...any) error {
223238
return &BoulderError{
224239
Type: RateLimit,
225240
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-registered-domain", args...),
226241
RetryAfter: retryAfter,
227242
}
228243
}
229244

230-
func CertificatesPerFQDNSetError(retryAfter time.Duration, msg string, args ...interface{}) error {
245+
func CertificatesPerFQDNSetError(retryAfter time.Duration, msg string, args ...any) error {
231246
return &BoulderError{
232247
Type: RateLimit,
233248
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-exact-set-of-hostnames", args...),
234249
RetryAfter: retryAfter,
235250
}
236251
}
237252

238-
func FailedAuthorizationsPerDomainPerAccountError(retryAfter time.Duration, msg string, args ...interface{}) error {
253+
func FailedAuthorizationsPerDomainPerAccountError(retryAfter time.Duration, msg string, args ...any) error {
239254
return &BoulderError{
240255
Type: RateLimit,
241256
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#authorization-failures-per-hostname-per-account", args...),
242257
RetryAfter: retryAfter,
243258
}
244259
}
245260

246-
func RejectedIdentifierError(msg string, args ...interface{}) error {
261+
func RejectedIdentifierError(msg string, args ...any) error {
247262
return newf(RejectedIdentifier, msg, args...)
248263
}
249264

250-
func InvalidEmailError(msg string, args ...interface{}) error {
265+
func InvalidEmailError(msg string, args ...any) error {
251266
return newf(InvalidEmail, msg, args...)
252267
}
253268

254-
func UnsupportedContactError(msg string, args ...interface{}) error {
269+
func UnsupportedContactError(msg string, args ...any) error {
255270
return newf(UnsupportedContact, msg, args...)
256271
}
257272

258-
func ConnectionFailureError(msg string, args ...interface{}) error {
273+
func ConnectionFailureError(msg string, args ...any) error {
259274
return newf(ConnectionFailure, msg, args...)
260275
}
261276

262-
func CAAError(msg string, args ...interface{}) error {
277+
func CAAError(msg string, args ...any) error {
263278
return newf(CAA, msg, args...)
264279
}
265280

266-
func MissingSCTsError(msg string, args ...interface{}) error {
281+
func MissingSCTsError(msg string, args ...any) error {
267282
return newf(MissingSCTs, msg, args...)
268283
}
269284

270-
func DuplicateError(msg string, args ...interface{}) error {
285+
func DuplicateError(msg string, args ...any) error {
271286
return newf(Duplicate, msg, args...)
272287
}
273288

274-
func OrderNotReadyError(msg string, args ...interface{}) error {
289+
func OrderNotReadyError(msg string, args ...any) error {
275290
return newf(OrderNotReady, msg, args...)
276291
}
277292

278-
func DNSError(msg string, args ...interface{}) error {
293+
func DNSError(msg string, args ...any) error {
279294
return newf(DNS, msg, args...)
280295
}
281296

282-
func BadPublicKeyError(msg string, args ...interface{}) error {
297+
func BadPublicKeyError(msg string, args ...any) error {
283298
return newf(BadPublicKey, msg, args...)
284299
}
285300

286-
func BadCSRError(msg string, args ...interface{}) error {
301+
func BadCSRError(msg string, args ...any) error {
287302
return newf(BadCSR, msg, args...)
288303
}
289304

290-
func AlreadyReplacedError(msg string, args ...interface{}) error {
305+
func AlreadyReplacedError(msg string, args ...any) error {
291306
return newf(AlreadyReplaced, msg, args...)
292307
}
293308

294-
func AlreadyRevokedError(msg string, args ...interface{}) error {
309+
func AlreadyRevokedError(msg string, args ...any) error {
295310
return newf(AlreadyRevoked, msg, args...)
296311
}
297312

@@ -303,18 +318,18 @@ func UnknownSerialError() error {
303318
return newf(UnknownSerial, "unknown serial")
304319
}
305320

306-
func InvalidProfileError(msg string, args ...interface{}) error {
321+
func InvalidProfileError(msg string, args ...any) error {
307322
return newf(InvalidProfile, msg, args...)
308323
}
309324

310-
func BadSignatureAlgorithmError(msg string, args ...interface{}) error {
325+
func BadSignatureAlgorithmError(msg string, args ...any) error {
311326
return newf(BadSignatureAlgorithm, msg, args...)
312327
}
313328

314-
func AccountDoesNotExistError(msg string, args ...interface{}) error {
329+
func AccountDoesNotExistError(msg string, args ...any) error {
315330
return newf(AccountDoesNotExist, msg, args...)
316331
}
317332

318-
func BadNonceError(msg string, args ...interface{}) error {
333+
func BadNonceError(msg string, args ...any) error {
319334
return newf(BadNonce, msg, args...)
320335
}

probs/probs.go

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ type SubProblemDetails struct {
7070
Identifier identifier.ACMEIdentifier `json:"identifier"`
7171
}
7272

73-
func (pd *ProblemDetails) Error() string {
73+
func (pd *ProblemDetails) String() string {
7474
return fmt.Sprintf("%s :: %s", pd.Type, pd.Detail)
7575
}
7676

@@ -329,26 +329,6 @@ func Conflict(detail string) *ProblemDetails {
329329
}
330330
}
331331

332-
// ContentLengthRequired returns a ProblemDetails representing a missing
333-
// Content-Length header error
334-
func ContentLengthRequired() *ProblemDetails {
335-
return &ProblemDetails{
336-
Type: MalformedProblem,
337-
Detail: "missing Content-Length header",
338-
HTTPStatus: http.StatusLengthRequired,
339-
}
340-
}
341-
342-
// InvalidContentType returns a ProblemDetails suitable for a missing
343-
// ContentType header, or an incorrect ContentType header
344-
func InvalidContentType(detail string) *ProblemDetails {
345-
return &ProblemDetails{
346-
Type: MalformedProblem,
347-
Detail: detail,
348-
HTTPStatus: http.StatusUnsupportedMediaType,
349-
}
350-
}
351-
352332
// MethodNotAllowed returns a ProblemDetails representing a disallowed HTTP
353333
// method error.
354334
func MethodNotAllowed() *ProblemDetails {

probs/probs_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func TestProblemDetails(t *testing.T) {
1515
Detail: "Wat? o.O",
1616
HTTPStatus: 403,
1717
}
18-
test.AssertEquals(t, pd.Error(), "malformed :: Wat? o.O")
18+
test.AssertEquals(t, pd.String(), "malformed :: Wat? o.O")
1919
}
2020

2121
func TestProblemDetailsConvenience(t *testing.T) {

ra/ra_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ func TestPerformValidationVAError(t *testing.T) {
851851
challenge, err := bgrpc.PBToChallenge(dbAuthzPB.Challenges[challIdx])
852852
test.AssertNotError(t, err, "Failed to marshall corepb.Challenge to core.Challenge.")
853853
test.Assert(t, challenge.Status == core.StatusInvalid, "challenge was not marked as invalid")
854-
test.AssertContains(t, challenge.Error.Error(), "Could not communicate with VA")
854+
test.AssertContains(t, challenge.Error.String(), "Could not communicate with VA")
855855
test.Assert(t, challenge.ValidationRecord == nil, "challenge had a ValidationRecord")
856856

857857
// Check that validated timestamp was recorded, stored, and retrieved

va/caa.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package va
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"net/url"
78
"regexp"
@@ -70,7 +71,7 @@ func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAVal
7071
if prob != nil {
7172
// CAA check failed.
7273
probType = string(prob.Type)
73-
logEvent.Error = prob.Error()
74+
logEvent.Error = prob.String()
7475
} else {
7576
// CAA check passed.
7677
outcome = pass
@@ -145,7 +146,7 @@ func (va *ValidationAuthorityImpl) checkCAA(
145146
identifier identifier.ACMEIdentifier,
146147
params *caaParams) error {
147148
if core.IsAnyNilOrZero(params, params.validationMethod, params.accountURIID) {
148-
return probs.ServerInternal("expected validationMethod or accountURIID not provided to checkCAA")
149+
return errors.New("expected validationMethod or accountURIID not provided to checkCAA")
149150
}
150151

151152
foundAt, valid, response, err := va.checkCAARecords(ctx, identifier, params)

va/caa_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1139,7 +1139,7 @@ func TestCAAFailure(t *testing.T) {
11391139
}
11401140
prob := detailedError(err)
11411141
test.AssertEquals(t, prob.Type, probs.DNSProblem)
1142-
test.AssertContains(t, prob.Error(), "NXDOMAIN")
1142+
test.AssertContains(t, prob.String(), "NXDOMAIN")
11431143
}
11441144

11451145
func TestFilterCAA(t *testing.T) {

va/dns_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func TestDNSValidationWrong(t *testing.T) {
2424
t.Fatalf("Successful DNS validation with wrong TXT record")
2525
}
2626
prob := detailedError(err)
27-
test.AssertEquals(t, prob.Error(), "unauthorized :: Incorrect TXT record \"a\" found at _acme-challenge.wrong-dns01.com")
27+
test.AssertEquals(t, prob.String(), "unauthorized :: Incorrect TXT record \"a\" found at _acme-challenge.wrong-dns01.com")
2828
}
2929

3030
func TestDNSValidationWrongMany(t *testing.T) {
@@ -35,7 +35,7 @@ func TestDNSValidationWrongMany(t *testing.T) {
3535
t.Fatalf("Successful DNS validation with wrong TXT record")
3636
}
3737
prob := detailedError(err)
38-
test.AssertEquals(t, prob.Error(), "unauthorized :: Incorrect TXT record \"a\" (and 4 more) found at _acme-challenge.wrong-many-dns01.com")
38+
test.AssertEquals(t, prob.String(), "unauthorized :: Incorrect TXT record \"a\" (and 4 more) found at _acme-challenge.wrong-many-dns01.com")
3939
}
4040

4141
func TestDNSValidationWrongLong(t *testing.T) {
@@ -46,7 +46,7 @@ func TestDNSValidationWrongLong(t *testing.T) {
4646
t.Fatalf("Successful DNS validation with wrong TXT record")
4747
}
4848
prob := detailedError(err)
49-
test.AssertEquals(t, prob.Error(), "unauthorized :: Incorrect TXT record \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...\" found at _acme-challenge.long-dns01.com")
49+
test.AssertEquals(t, prob.String(), "unauthorized :: Incorrect TXT record \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...\" found at _acme-challenge.long-dns01.com")
5050
}
5151

5252
func TestDNSValidationFailure(t *testing.T) {

va/tlsalpn_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func TestTLSALPN01TalkingToHTTP(t *testing.T) {
254254
test.AssertError(t, err, "TLS-SNI-01 validation passed when talking to a HTTP-only server")
255255
prob := detailedError(err)
256256
expected := "Server only speaks HTTP, not TLS"
257-
if !strings.HasSuffix(prob.Error(), expected) {
257+
if !strings.HasSuffix(prob.String(), expected) {
258258
t.Errorf("Got wrong error detail. Expected %q, got %q", expected, prob)
259259
}
260260
}
@@ -780,7 +780,7 @@ func TestTLSALPN01ExtraSANs(t *testing.T) {
780780
// In go >= 1.19, the TLS client library detects that the certificate has
781781
// a duplicate extension and terminates the connection itself.
782782
prob := detailedError(err)
783-
test.AssertContains(t, prob.Error(), "Error getting validation data")
783+
test.AssertContains(t, prob.String(), "Error getting validation data")
784784
}
785785

786786
func TestTLSALPN01ExtraAcmeExtensions(t *testing.T) {
@@ -795,7 +795,7 @@ func TestTLSALPN01ExtraAcmeExtensions(t *testing.T) {
795795
// In go >= 1.19, the TLS client library detects that the certificate has
796796
// a duplicate extension and terminates the connection itself.
797797
prob := detailedError(err)
798-
test.AssertContains(t, prob.Error(), "Error getting validation data")
798+
test.AssertContains(t, prob.String(), "Error getting validation data")
799799
}
800800

801801
func TestAcceptableExtensions(t *testing.T) {
@@ -857,7 +857,7 @@ func TestTLSALPN01BadIdentifier(t *testing.T) {
857857
_, err := va.validateTLSALPN01(ctx, identifier.ACMEIdentifier{Type: "smime", Value: "dobber@bad.horse"}, expectedKeyAuthorization)
858858
test.AssertError(t, err, "Server accepted a hypothetical S/MIME identifier")
859859
prob := detailedError(err)
860-
test.AssertContains(t, prob.Error(), "Identifier type for TLS-ALPN-01 challenge was not DNS or IP")
860+
test.AssertContains(t, prob.String(), "Identifier type for TLS-ALPN-01 challenge was not DNS or IP")
861861
}
862862

863863
// TestTLSALPN01ServerName tests compliance with RFC 8737, Sec. 3 (step 3) & RFC

va/va.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ func (va *ValidationAuthorityImpl) DoDCV(ctx context.Context, req *vapb.PerformV
703703
outcome := fail
704704
if prob != nil {
705705
probType = string(prob.Type)
706-
logEvent.Error = prob.Error()
706+
logEvent.Error = prob.String()
707707
logEvent.Challenge.Error = prob
708708
logEvent.Challenge.Status = core.StatusInvalid
709709
} else {

0 commit comments

Comments
 (0)