[PR #5163] [client] Try next DNS upstream on SERVFAIL/REFUSED responses #27235

Closed
opened 2026-08-05 07:08:24 -04:00 by saavagebueno · 0 comments
Owner

Original Pull Request: https://github.com/netbirdio/netbird/pull/5163

State: closed
Merged: Yes


Describe your changes

Previously, we would only try the next upstream server if there was no response/timeout or any other query error.
Instead, we should also try the next upstream if the queried server reported issues.
This PR also improves logging for upstream failures.

Example when one fails (end result = success):


2026-01-23T10:25:34Z WARN [request_id: 06c63d15, dns_id: 8af7] client/internal/dns/upstream.go:239: 1/2 upstreams failed for domain=test.servfail.: 127.0.0.1:5353=SERVFAIL
2026-01-23T10:25:34Z TRAC [request_id: 06c63d15, dns_id: 8af7] client/internal/dns/handler_chain.go:264: response: domain=test.servfail. rcode=NOERROR answers=[192.0.2.200] upstream=127.0.0.1:5354 took=1.393757ms

Example when all fail:


2026-01-23T10:28:29Z ERRO [request_id: 3d5d0b4f, dns_id: 6468] client/internal/dns/upstream.go:241: 2/2 upstreams failed for domain=test.servfail.: 127.0.0.1:5353=SERVFAIL, 127.0.0.1:5354=SERVFAIL
2026-01-23T10:28:29Z TRAC [request_id: 3d5d0b4f, dns_id: 6468] client/internal/dns/handler_chain.go:264: response: domain=test.servfail. rcode=SERVFAIL answers=[] took=1.267542ms
█

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced DNS request logging with additional contextual information for improved debugging.
    • Improved upstream server failover logic with per-server failure tracking and detailed failure summaries.
  • Bug Fixes

    • DNS error responses now only returned when all upstream servers fail, preventing premature error responses.
  • Tests

    • Added comprehensive test coverage for DNS failover scenarios and failure handling.

✏️ Tip: You can customize this high-level summary in your review settings.

**Original Pull Request:** https://github.com/netbirdio/netbird/pull/5163 **State:** closed **Merged:** Yes --- ## Describe your changes Previously, we would only try the next upstream server if there was no response/timeout or any other query error. Instead, we should also try the next upstream if the queried server reported issues. This PR also improves logging for upstream failures. Example when one fails (end result = success): ``` 2026-01-23T10:25:34Z WARN [request_id: 06c63d15, dns_id: 8af7] client/internal/dns/upstream.go:239: 1/2 upstreams failed for domain=test.servfail.: 127.0.0.1:5353=SERVFAIL 2026-01-23T10:25:34Z TRAC [request_id: 06c63d15, dns_id: 8af7] client/internal/dns/handler_chain.go:264: response: domain=test.servfail. rcode=NOERROR answers=[192.0.2.200] upstream=127.0.0.1:5354 took=1.393757ms ``` Example when all fail: ``` 2026-01-23T10:28:29Z ERRO [request_id: 3d5d0b4f, dns_id: 6468] client/internal/dns/upstream.go:241: 2/2 upstreams failed for domain=test.servfail.: 127.0.0.1:5353=SERVFAIL, 127.0.0.1:5354=SERVFAIL 2026-01-23T10:28:29Z TRAC [request_id: 3d5d0b4f, dns_id: 6468] client/internal/dns/handler_chain.go:264: response: domain=test.servfail. rcode=SERVFAIL answers=[] took=1.267542ms █ ``` ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Enhanced DNS request logging with additional contextual information for improved debugging. * Improved upstream server failover logic with per-server failure tracking and detailed failure summaries. * **Bug Fixes** * DNS error responses now only returned when all upstream servers fail, preventing premature error responses. * **Tests** * Added comprehensive test coverage for DNS failover scenarios and failure handling. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
saavagebueno added the pull-request label 2026-08-05 07:08:24 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#27235