Skip to content

Alternative DirProvider API can force provider to lie about errors

In !347 (merged) we just added an API to allow an application to provide an alternative to our dirmgr, via the new DirProvider trait. There are callbacks made into the application's code, and some can return errors: the builder requires the hook to provide an arti_client::Error; and the bootstrap method on the provider requires the hook to provide a tor_dirmgr::Error.

Currently, these types cannot contain errors we have not foreseen. So if the alternative directory provider needs to report some other kind of error, they must, basically, lie, by providing an enum variant containing, perhaps, nonsense. I find this situation unconscionable, even for an extension API for expert users.

We can try to fix this problem by asking the API provider to return some other error type. But of course, we want the API to be the one that dirmgr also provides.

Also, whatever we do, the error from the directory provider is going to end up in a type that impl HasKind. This means that the directory provider must choose an ErrorKind. But perhaps none of our ErrorKinds are right. Again, the directory provider might be forced, by our API, to lie.

We do not know what the provider's own error type(s) may be like. We don't want to make everything monomorphisedly-generic over the provider's error type. So I think we need to store the caller's actual error as Box<dyn ...> or Arc<dyn ...>.

As for the kind, we need to provide kinds that can be used for situations we haven't foreseen. I think we still need it to imply a location - which rules out a single kind like ErrorKind::FromExternalDirProvider.

I therefore propose the following:

  • Add a variant to tor_dirmgr::Error, FromExternalDirProvider. It will contain one of:

    • Arc<dyn tor_error::ErrorWithKind> , a new trait which : HasKind + StdError ....
    • Arc<dyn StdError + Send + Sync + 'static> + a separate ErrorKind
    • Arc<dyn HasKind + Send + Sync + 'static> and we make HasKind: std::error::Error

  • Add the following ErrorKinds, for use only by code outside arti.git: OtherLocal OtherTorAccess OtherTorNetwork OtherRemote OtherIndeterminate or maybe some other word than Other. We could have ExternalDirProviderLocal etc. but that seems overly pernickety.

Earlier discussion: !347 (comment 2782765)

CC @eta, since I know she especially objected to Arc<dyn...>. I'm suggesting this for this situation because I can't see a reasonable alternative.