I’ve been refining the API for libdav. Two big tasks were involved here: (1)
more flexible generic types for the async helpers and (2) tidying up error
types.
Flexible async types
The core of libdav follows a sans I/O philosophy: it produces
requests and parses response, leaving network transport up to the caller. This
kind of design is clean, flexible, and especially convenient for unit testing.
Not to mention that it makes the library actually usable with any HTTP client
implementation.
For typical consumers writing async Rust code with the hyper HTTP library,
writing code to use a sans-io library is somewhat tedious, so libdav provides
the necessary abstractions for this too, as an upper layer.
The existing implementation could be used with any client which returned a regular HTTP response. It never occurred to me, but this mean it’s unusable with a client that returns a compressed HTTP response, and using compressing for WebDAV (a text-based protocol) provides immense value.1
I adapted the async code to be more flexible, and accept any clients which
returns something which can be converted into an HTTP response, enabling the
use of compression and likely some other niche cases. I also updated pimsync
to use HTTP compression when available, substantially reducing the amount of
network traffic involved.
Error types in Rust
Regarding the error types: I have mixed feelings about this kind of work. All
error types are much cleaner. Handling and analysing errors on upper layers
(e.g.: consumers of libdav) should be much clearer now.
It feels useful, yet if an end-user of pimsync asks “what improved”, the answer is “nothing”, and that’s exactly the source of my mixed feelings. It’s not even that failure scenarios would behave different for end users — beyond slightly clearer error messages in some niche situations.
And I feel that Rust lends itself to this: its type system is so rich that we can refine upon a design infinitely, always improving. It’s a potentially unbound time sink, hence my mixed feelings around it.
Actual changes in error type refinement
The previous design basically had every function wrap the errors returned by the functions it called. Error types reflected the whole function call tree. In theory this allows a caller to discern exactly what went wrong, but in practice, these were bloated error types which had too many variants to realistically handle in any way beyond just logging them.
This design made debugging much clearer, but debugging through error types is using the wrong tool and the wrong approach.
I’ve been wanting to clean up this design for a while, especially since all the surrounding code also gets messy and noisy very quickly.
A common pattern is Rust is for a library to have a single error type for
everything. I find this to be an anti-pattern: functions will return error types
with unreachable variants, and when handling these in detail, we end up with a
lot of => unreachable!() statements because the signature declares return
variants which it can never return. A single error type is a no-go for any
non-trivial library.
Inspired by Designing error types in Rust, I set out to clean up all this mess and provide error types with fewer variants, and which each clearly explain what went wrong… but not in too much detail!
For example, let’s look at TxtError, the error type returned when resolving a
context path via a DNS TXT record:
/// Error returned by [`find_context_path_via_txt_records`].
#[derive(thiserror::Error, Debug)]
pub enum TxtError {
/// I/O error performing DNS request.
#[error("I/O error performing DNS request: {0}")]
Network(#[from] io::Error),
/// Domain name is too long.
#[error("domain name is too long and cannot be queried: {0}")]
DomainTooLong(#[from] LongChainError),
/// Error parsing DNS response.
#[error("parsing DNS response: {0}")]
ParseError(#[from] ParseError),
/// DNS response contained data invalid for building a URL path.
#[error("invalid data in response: {0}")]
InvalidData(#[from] InvalidUri),
/// DNS response is missing the expected prefix `path=`.
#[error("missing expected prefix path= from TXT record.")]
BadTxt,
}
Each error variant wraps the inner error type for each potential branch in the code that could fail. Granular, but too granular: the last three variants are basically the same error: “data in the DNS record isn’t valid”. Additionally, it’s leaking the inner implementation details into the public signature. The replacement is more concise:
/// Error performing a DNS lookup as part of service discovery.
///
/// Returned by [`resolve_srv_record`] and [`find_context_path_via_txt_records`].
#[derive(thiserror::Error, Debug)]
pub enum DnsError {
/// Input domain name is too long to be queried for this service.
#[error("domain name is too long and cannot be queried")]
DomainTooLong,
/// DNS query could not be performed.
#[error("performing DNS query: {0}")]
Query(#[from] io::Error),
/// DNS records for this service are malformed.
#[error("malformed DNS record: {0}")]
MalformedRecord(Box<dyn std::error::Error + Send + Sync>),
}
Note that this same error type is now re-used by two functions: one which
fetches data from SRV records and another which fetches data from TXT records.
Their error modes are the same — errors fetching data from a DNS record — so
they both get folded into one. DomainTooLong no longer wraps the inner error;
the error variant is clear enough and no further data is required.
And, of importance, the three error variants reflect different conditions:
DomainTooLong: basically invalid input, or impossible to fulfil.Query: network error, potentially transient.MalformedRecord: data was garbage, tell the administrator to fix it.
Likewise, ParseResponseError collapsed its
InvalidStatusCode(InvalidStatusCode) and NotUtf8(std::str::Utf8Error)
variants into a single
InvalidResponse(Box<dyn std::error::Error + Send + Sync>) error. The heap
allocation for that last error low-key bothers me, but it’s insignificant next
to a DNS network round-trip, and the inner type is likely only going to be used
to generate a string for logging / UI purposes.2
Getting error types in Rust is tricky, and a library needs to keep several things in mind:
- Provide clear granularity: users might want to handle different errors via different paths.
- But not too much granularity: otherwise things become unwieldy.
- Avoid leaking implementation details: otherwise signature will diverge over time (or will become a burden).
- Don’t return errors with unreachable variants: this makes handling noisy and confusing.
Encryption was always supported; the client handles encryption transparently and yields a regular response. It’s only compression that has special treatment in the current ecosystem. ↩︎
This can likely be refined further, but I fear that I could keep saying this forever, no matter how much further I refine. ↩︎