Conversation
revokeAuthorizationChallenges() runs before authorize() on every renewal and also calls new-order, so a Let's Encrypt rate limit surfaced there as an uncaught RateLimitedServerException: the new rate-limit message was never shown and `ssl-renew --all` aborted on that site. Stop revoking on a rate limit and let authorize() report it.
The fallback matched any error containing "too many" (e.g. "Too many open files"), which would be reported as a Let's Encrypt rate limit. acmephp maps every `rateLimited` ACME error to RateLimitedServerException, so the class check plus the `rateLimited` marker is enough. Also correct the docblock, which claimed HTTP 429 detection the code never did.
…crypt The jitter slept before every dispatched LE site in `ssl-renew --all`, including the sites whose certificates aren't due, which make no ACME calls. On the daily cron that added about 3s per site for nothing (roughly 5 minutes for 100 sites) while holding up the run. Move it into renew_ssl_cert() after the renewal-necessity check, so it only spaces out real renewals within one process.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ssl-renew --allfires per-site ACME operations back-to-back with no spacing, and on a Let's Encrypt rate limitauthorize()shows a misleading "you're in a local environment or using a non-public domain" message instead of saying it's a rate limit.Fix
renew_ssl_cert(), after the "is renewal due / --force" check, asleep( random_int( 1, 5 ) )before every real renewal except the first in the process, to avoid bursting against Let's Encrypt's new-order/burst limits. Sites that aren't due (almost all of them on a normal night) and single-siteee site ssl-renew <site>never sleep. The flag is a process-wide static because--allrenews every site in one process.is_rate_limit_exception()detects rate limits viaRateLimitedServerException(acmephp maps everyrateLimitedACME problem to it), plus arateLimitedmarker in the message.authorize()and bothexecuteRenewal()catches then say "Let's Encrypt rate limit hit for: … Please wait before retrying." with the rate-limits docs link. A rate limit hit while revoking the previous authorizations (the first new-order of every renewal) is now caught there too, so the renewal reports the rate limit instead of crashing.With this change a rate-limited site returns false and
ssl-renew --allcontinues. Other uncaught exceptions orEE::error()in one site still abort the whole batch (pre-existing; out of scope).Out of scope (follow-ups)
A fully failure-resilient
--allwith automatic backoff-and-retry, and removing the deadif ( $all )branch inssl_renew(), are larger changes left for later.Note for merge
This edits
executeRenewal()'s catch blocks, which #486 also edits — whichever merges second will need a small rebase to reconcile.Testing
Manual:
ssl-renew --allwith several LE sites that aren't due runs without any sleep; with two or more due sites there is a 1–5s pause before each renewal after the first. The rate-limit path was verified against acmephp with a mocked 429rateLimitedresponse (bothauthorize()and the revoke-then-authorize renewal flow print the rate-limit message and return false); unrelated errors such as "Too many open files" orrejectedIdentifierkeep their own messages.Tested on Ubuntu 26.04 with EasyEngine 4.12.0 with five sites flagged as Let's Encrypt (three not due, two due), and on PHP 7.4 and 8.5 for the rate-limit handling.