try_from_* methods should either return an error-containing Result or an Option

[fralalonde]

Considering the following DNSNameRef signatures

pub fn try_from_ascii(dns_name: untrusted::Input<'a>) -> Result<Self, ()> {
pub fn try_from_ascii_str(dns_name: &str) -> Result<DNSNameRef, ()> {

From an API standpoint, I think returning a Result that has () for an error type is not good design.

If multiple libraries adopt this pattern, it is not possible to differentiate between the () from lib A and () from lib B. Which forces any error to be map_err() / handled immediately by the calling code when most often a simple ? would be preferable. It would otherwise be preposterous to force applications to assume that any () error result originates only from webpki, or to require that any error of type () be considered of unknown origin.

If a Result<> has no error context to provide, an error-less Option<> should be returned instead.

[briansmith]

It seems like these functions should have an error type like struct NotADNSName; Then people can write their From<NotADNSName> conversions to the more specific errors.

Error is used instead of Option because we're trying to follow the gist of TryFrom from rust-lang/rust#33417. My plan is, once TryFrom is available, to switch to using TryFrom for these versions.

[briansmith]

Note that in this crate and in ring we avoid most error infrastructure. In particular, we avoid implementing std::error::Error (it's poorly designed) and we avoid using the failure and error_chain crates intentionally (their too heavyweight and I generally avoid macros when practical).

[fralalonde]

These choices and explanations are perfectly reasonable. Too bad TryFrom has been pushed back.

Introducing NotADNSName as suggested would provide a good interim solution.

[briansmith]

Introducing NotADNSName as suggested would provide a good interim solution.

I would take a PR that does this.

[fralalonde]

I followed the idea you expressed, let me know what you think.