Skip to content

Commit

Permalink
Revert "wfe: on rate limit error, serve 500 (#7796)"
Browse files Browse the repository at this point in the history
This reverts commit 242d746.

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`).
  • Loading branch information
jsha committed Dec 18, 2024
1 parent 0c658f2 commit ec5776b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 8 deletions.
7 changes: 6 additions & 1 deletion web/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
14 changes: 7 additions & 7 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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()
}
}

Expand Down

0 comments on commit ec5776b

Please sign in to comment.