Skip to content

dns error tracing#18247

Closed
icing wants to merge 3 commits intocurl:masterfrom
icing:dns-error-msgs
Closed

dns error tracing#18247
icing wants to merge 3 commits intocurl:masterfrom
icing:dns-error-msgs

Conversation

@icing
Copy link
Contributor

@icing icing commented Aug 11, 2025

  • Add more tracing information to c-ares errors.
  • remove CURL_ASYNC_SUCCESS, rename ares->last_status to ares->ares_status. Give trace explanation for "common" errors
  • add ares "csv" information to tracing on failure
  • DoH: invoke Curl_resolver_error() on failure to populate error buf

* Add more tracing information to c-ares errors.
* remove CURL_ASYNC_SUCCESS, rename `ares->last_status` to
  `ares->ares_status`. Give trace explanation for "common"
  errors
* add ares "csv" information to tracing on failure
* DoH: invoke `Curl_resolver_error()` on failure to populate
  error buf
@github-actions github-actions bot added the name lookup DNS and related tech label Aug 11, 2025
@bagder bagder closed this in 9cc4e24 Aug 11, 2025
Copy link
Contributor

@MichalPetryka MichalPetryka left a comment

Choose a reason for hiding this comment

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

I think that more info should be added to normal error messages too, not just trace since stuff like whether address doesn't exist or if DNS server isn't responding matter to normal users too, not just devs and even web browsers show that info by default.

Comment on lines +363 to +380
const char *msg;
switch(ares->ares_status) {
case ARES_ECONNREFUSED:
msg = "connection to DNS server refused";
break;
case ARES_ETIMEOUT:
msg = "query to DNS server timed out";
break;
case ARES_ENOTFOUND:
msg = "DNS server did not find the address";
break;
case ARES_EREFUSED:
msg = "DNS server refused query";
break;
default:
msg = "resolve failed";
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to use ares_strerror here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. Looked for such a function, but initially failed. My bad.

@icing
Copy link
Contributor Author

icing commented Aug 11, 2025

Just made #18251 where the error string is retrieved from c-ares and added to the "normal" failure message. So, curl users with c-ares can see the reason a resolve failed without doing anything special.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

name lookup DNS and related tech

Development

Successfully merging this pull request may close these issues.

3 participants

Comments