From ec5776b0ac45fe6bf74a6fdf562686d0efaecac8 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 18 Dec 2024 10:53:24 -0800 Subject: [PATCH] Revert "wfe: on rate limit error, serve 500 (#7796)" This reverts commit 242d74604034a0204c00ebdf3188c6eac53d508f. We want to make this change, but it carries some risk that we'd prefer not to take over the holiday. We'd also like to keep `main` in a state where it would be reasonable to deploy (even if, in practice, any over-the-holiday deploy would be a hotfix, not a direct tag from `main`). --- web/context.go | 7 ++++++- wfe2/wfe.go | 14 +++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/web/context.go b/web/context.go index a748137a0aa..36cd28bbf29 100644 --- a/web/context.go +++ b/web/context.go @@ -35,7 +35,12 @@ type RequestEvent struct { Slug string `json:",omitempty"` InternalErrors []string `json:",omitempty"` Error string `json:",omitempty"` - UserAgent string `json:"ua,omitempty"` + // If there is an error checking the data store for our rate limits + // we ignore it, but attach the error to the log event for analysis. + // TODO(#7796): Treat errors from the rate limit system as normal + // errors and put them into InternalErrors. + IgnoredRateLimitError string `json:",omitempty"` + UserAgent string `json:"ua,omitempty"` // Origin is sent by the browser from XHR-based clients. Origin string `json:",omitempty"` Extra map[string]interface{} `json:",omitempty"` diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 19063b7cd2f..7f5ce1f812f 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -798,8 +798,7 @@ func (wfe *WebFrontEndImpl) NewAccount( wfe.sendError(response, logEvent, probs.RateLimited(err.Error()), err) return } else { - wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "While checking rate limits"), err) - return + logEvent.IgnoredRateLimitError = err.Error() } } @@ -2407,13 +2406,14 @@ func (wfe *WebFrontEndImpl) NewOrder( } refundLimits, err := wfe.checkNewOrderLimits(ctx, acct.ID, names, isRenewal) - if err != nil && features.Get().UseKvLimitsForNewOrder { + if err != nil { if errors.Is(err, berrors.RateLimit) { - wfe.sendError(response, logEvent, probs.RateLimited(err.Error()), err) - return + if features.Get().UseKvLimitsForNewOrder { + wfe.sendError(response, logEvent, probs.RateLimited(err.Error()), err) + return + } } else { - wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "While checking rate limits"), err) - return + logEvent.IgnoredRateLimitError = err.Error() } }