Refactor logging and error handling #60
Merged
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR refactors logging and error handling throughout the ACME certificate management system to provide better error messages, proper error categorization, and improved handling of rate limits and retry scenarios.
- Introduces structured error types to replace generic error handling
- Adds detailed logging for certificate operations and account management
- Implements proper rate limit handling with server-provided retry intervals
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/state/issuer.rs | Adds error state tracking and exponential backoff for issuer failures |
| src/state/certificate.rs | Refactors certificate validity checking with separate is_valid() method |
| src/lib.rs | Replaces generic error handling with structured error types and detailed logging |
| src/conf/order.rs | Refactors order identification with new PrintableOrderId type |
| src/conf/issuer.rs | Adds issuer-level error handling method |
| src/acme/error.rs | Introduces comprehensive error type hierarchy for ACME operations |
| src/acme.rs | Major refactoring of ACME client with structured error handling and improved logging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bavshin-f5
force-pushed
the
error-handling
branch
from
e3dffdb to
b5a18a7
Compare
bavshin-f5
force-pushed
the
error-handling
branch
2 times, most recently
from
3f46ef6 to
003a368
Compare
|
Example messages: |
Also, prefer first available DNS name.
bavshin-f5
force-pushed
the
error-handling
branch
from
003a368 to
5593251
Compare
Clean up error handling in the main update loop.
* Implement backoff for login errors * Log new account URLs
We already log the URL, but on INFO level and only once. And while it is possible to find the URL from private key, the operation is not trivial. Let's make this simpler by writing a file named "account.url" to the state directory whenever the issuer tells us it created a new account. Fixes nginx#54
Previously, it was possible to suspend the module execution by sending a sufficiently high Retry-After value. Now we will refuse to wait longer than one minute.
bavshin-f5
force-pushed
the
error-handling
branch
from
5593251 to
8dd01b0
Compare
We don't care about the error kind here, we just need to pass it to the caller and print. anyhow seems a bit too heavy for the task. While we're at it, remove unused futures-channel.
bavshin-f5
force-pushed
the
error-handling
branch
from
8dd01b0 to
527e7a7
Compare
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.
This has been slowly progressing since before the initial release, and while I'm not completely satisfied, it is time to publish something.
Fixes lots of issues with insufficient or just plain bad log messages, incorrect error reporting etc.
Also, fixes handling of rate limit errors and large Retry-After values.
I'm still planning to review and adjust the individual messages while testing this.