-
-
Notifications
You must be signed in to change notification settings - Fork 108
fix missing MX resolver eg. on android #2852
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
Conversation
switching completely to /etc/resolv.conf (see #2780) does not work at least on some Androids (see deltachat/deltachat-android#2151 ) therefore, we use the old approach as a fallback.
5d9f2fb
to
39bc95c
Compare
39bc95c
to
1297ed0
Compare
/// This does not work at least on some Androids, therefore we use use ResolverConfig::default() | ||
/// which default eg. to google's 8.8.8.8 or 8.8.4.4 as a fallback. | ||
async fn get_resolver() -> Result<AsyncStdResolver, ResolveError> { | ||
if let Ok(resolver) = resolver_from_system_conf().await { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log the error here?
/// We first try resolver_from_system_conf() which reads the system's resolver from `/etc/resolv.conf`. | ||
/// This does not work at least on some Androids, therefore we use use ResolverConfig::default() | ||
/// which default eg. to google's 8.8.8.8 or 8.8.4.4 as a fallback. | ||
async fn get_resolver() -> Result<AsyncStdResolver, ResolveError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok to simply import anyhow::Result
at the top of the module rather than the test and return it here, so anyhow::Result
is used as much as possible, preferrably across the whole crate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i also first tried that, getting "expected struct anyhow::Error
, found struct ResolveError
" - so i assumed i have to convert the object somehow and did not want to dive into that yesterday :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like
let resolver = ...?;
Ok(resolver)
?
will convert the Result
automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's clever! i did not thought too much about these conversions in the past :)
i did a successor pr that uses map_err() at #2853 - but you would prefer the one above?
Related issue in the upstream crate: https://github.com/bluejekyll/trust-dns/issues/652 |
switching completely to /etc/resolv.conf (see #2780)
does not work at least on some Androids
(see deltachat/deltachat-android#2151 )
therefore, we use the old approach as a fallback.
fixes deltachat/deltachat-android#2151
@link2xt i am not deep in that stuff, maybe there is a more elegant solution or sth. to tweak upstream, however, this one works :) (tested on android)
for review: the fix is in the first commit, the second one only adds a warning - and introduces some noise as we have to pass context around.