Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vec impl for DnsResolver #345

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions rama-dns/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,89 @@ impl<R: DnsResolver<Error: Into<BoxError>>> DnsResolver for Option<R> {
}
}

#[derive(Debug)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be a good idea to move all your new code here to a new file: rama-dns/src/chain.rs. Can be a private fail of which you pub use (re-export) types here.

pub struct DnsChainDomainResolveErr<E: 'static> {
errors: Vec<E>,
}

impl<E: std::fmt::Debug> std::fmt::Display for DnsChainDomainResolveErr<E> {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(
f,
"domain resolver chain resulted in errors: {:?}",
self.errors
)
}
}

impl<E: std::fmt::Debug + Send + std::error::Error> std::error::Error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do E: std::error::Error + 'static, can you not just do
self.errors.last() on line 101? If not what error do you get?

for DnsChainDomainResolveErr<E>
{
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.errors.last().map(|e| e as &dyn std::error::Error)
}
}

impl<R, E> DnsResolver for Vec<R>
where
R: DnsResolver<Error = E> + Send,
E: Send + 'static,
{
type Error = DnsChainDomainResolveErr<E>;

async fn ipv4_lookup(&self, domain: Domain) -> Result<Vec<Ipv4Addr>, Self::Error> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can refactor these 2 definitions as:

macro_rules! dns_resolver_chain_impl {
    () => {
        async fn ipv4_lookup(&self, domain: Domain) -> Result<Vec<Ipv4Addr>, Self::Error> {
            let mut errors = Vec::new();
            for resolver in self {
                match resolver.ipv4_lookup(domain.clone()).await {
                    Ok(ipv4s) => return Ok(ipv4s),
                    Err(err) => errors.push(err),
                }
            }
            Err(DnsChainDomainResolveErr { errors })
        }
    
        async fn ipv6_lookup(&self, domain: Domain) -> Result<Vec<Ipv6Addr>, Self::Error> {
            let mut errors = Vec::new();
            for resolver in self {
                match resolver.ipv6_lookup(domain.clone()).await {
                    Ok(ipv6s) => return Ok(ipv6s),
                    Err(err) => errors.push(err),
                }
            }
            Err(DnsChainDomainResolveErr { errors })
        }
    }
}

so that you can do:

impl<R, E> DnsResolver for Vec<R>
where
    R: DnsResolver<Error = E> + Send,
    E: Send + 'static,
{
    type Error = DnsChainDomainResolveErr<E>;
    
    dns_resolver_chain_impl!()
}

impl<R, E, const N: usize> DnsResolver for [R; N]
where
    R: DnsResolver<Error = E> + Send,
    E: Send + 'static,
{
    type Error = DnsChainDomainResolveErr<E>;
    
    dns_resolver_chain_impl!()
}

Might give you an error around scoping or w/e, but nothing that can be resolved by in worst case
passing some stuff. With some cleverness you can probably even make it shorter, but this is already sufficient.

let mut errors = Vec::new();
for resolver in self {
match resolver.ipv4_lookup(domain.clone()).await {
Ok(ipv4s) => return Ok(ipv4s),
Err(err) => errors.push(err),
}
}
Err(DnsChainDomainResolveErr { errors })
}

async fn ipv6_lookup(&self, domain: Domain) -> Result<Vec<Ipv6Addr>, Self::Error> {
let mut errors = Vec::new();
for resolver in self {
match resolver.ipv6_lookup(domain.clone()).await {
Ok(ipv6s) => return Ok(ipv6s),
Err(err) => errors.push(err),
}
}
Err(DnsChainDomainResolveErr { errors })
}
}

impl<R, E, const N: usize> DnsResolver for [R; N]
where
R: DnsResolver<Error = E> + Send,
E: Send + 'static,
{
type Error = DnsChainDomainResolveErr<E>;

async fn ipv4_lookup(&self, domain: Domain) -> Result<Vec<Ipv4Addr>, Self::Error> {
let mut errors = Vec::new();
for resolver in self {
match resolver.ipv4_lookup(domain.clone()).await {
Ok(ipv4s) => return Ok(ipv4s),
Err(err) => errors.push(err),
}
}
Err(DnsChainDomainResolveErr { errors })
}

async fn ipv6_lookup(&self, domain: Domain) -> Result<Vec<Ipv6Addr>, Self::Error> {
let mut errors = Vec::new();
for resolver in self {
match resolver.ipv6_lookup(domain.clone()).await {
Ok(ipv6s) => return Ok(ipv6s),
Err(err) => errors.push(err),
}
}
Err(DnsChainDomainResolveErr { errors })
}
}

macro_rules! impl_dns_resolver_either_either {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while you're at it, feel free to move this macro def and its usage to a separate file called rama-dns/src/variant.rs, keeps the lib.rs pretty clean

($id:ident, $($param:ident),+ $(,)?) => {
impl<$($param),+> DnsResolver for ::rama_core::combinators::$id<$($param),+>
Expand Down
Loading