proposal: net: add QueryType to *DNSError

[Skeeve]

Go version

go version go1.21.6 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/scripts/.cache/go-build'
GOENV='/home/scripts/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.6'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOMOD='/home/scripts/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1931870782=/tmp/go-build -gno-record-gcc-switches'

What did you do?

https://go.dev/play/p/oN0gVQA4nm2

Please Note: On goplay lookups don't work. You have to run it locally.

What did you see happen?

Host Lookup
[208.73.7.224]

TXT Lookup
[]
lookup mail10476.emails3.rightmove.co.uk on 192.168.65.7:53: no such host

What did you expect to see?

Instead of "no such host" I would hav expected "no TXT record"

See also

https://stackoverflow.com/questions/77880685/golang-net-lookuptxt-returns-no-such-host

[callthingsoff]

After a quick look at the code, a simple and ugly fix looks like this.

--- a/src/net/lookup.go
+++ b/src/net/lookup.go
@@ -856,6 +856,11 @@ func (r *Resolver) goLookupNS(ctx context.Context, name string) ([]*NS, error) {
 func (r *Resolver) goLookupTXT(ctx context.Context, name string) ([]string, error) {
        p, server, err := r.lookup(ctx, name, dnsmessage.TypeTXT, nil)
        if err != nil {
+               if e, ok := err.(*DNSError); ok {
+                       if e.Err == errNoSuchHost.Error() {
+                               e.Err = "no TXT record"
+                       }
+               }
                return nil, err
        }
        var txts []string

[Skeeve]

That wouldn't work. Please replace the hostname in my example with one that doesn't exist, e.g. put an "x" in front of the name.

Your "fix" would say that there is no such TXT record. While it technically is true, it's not consisternt with the host command:

$ host -t TXT xmail10476.emails3.rightmove.co.uk
Host xmail10476.emails3.rightmove.co.uk not found: 3(NXDOMAIN)

$ host -t TXT mail10476.emails3.rightmove.co.uk
mail10476.emails3.rightmove.co.uk has no TXT record

Is there any way to get the underlying DNS errors?

[mateusz834]

We currently return "no such host" for NXDOMAIN and NODATA conditions. Maybe we should change that and return "no such host" (or "no such domain") and "no records found"?
We might also introduce a QueryType field into the *net.DNSError, so that we can show the queried record type in the error message.

[Skeeve]

We currently return "no such host" for NXDOMAIN and NODATA conditions. Maybe we should change that and return "no such host" (or "no such domain") and "no records found"?

I think, this is a good idea.

In my current project I'm using Perl for the lookups I have to do. But I want to switch to go. Unfortunately I currently report the DNS errors to my customers and with the net module I will have to change the results. I don't think it's a big issue in my case, but at least an inconvenience.

We might also introduce a QueryType field into the *net.DNSError, so that we can show the queried record type in the error message.

This would be perfect.

[mateusz834]

Ok, lets make this a proposal then.

I propose adding a QueryType field to DNSError:

package net

type DNSError struct {
    // (...)
    QueryType string
}

func (e *DNSError) Error() string {
	if e == nil {
		return "<nil>"
	}
	s := "lookup: " 
        if e.QueryType != "" {
            s += e.QueryType + ": "
        }
        s += e.Name
	if e.Server != "" {
		s += " on " + e.Server
	}
	s += ": " + e.Err
	return s
}

The only question remains how to handle LookupHost errors, I think in that case it should be set to QueryType = "host" instead of A or AAAA, so that we don't have different behavior between platforms.

The "no such host"/"no records found" change does not require a proposal.

[kostix]

I would first make sure what @Skeeve's original wish regarding the stated problem was.
If it is purely about how errors of such class look when printed, — that is one thing; if, instead, they would like to be able to programmatically differentiate between NXDOMAIN and missing records of the queried class, — that's another story.

I have one more concern (maybe misguided): I would not say that having zero TXT records for an existing domain is an error to begin with. In the OP's case I would expect net.LookupTXT to return nil, nil or []string{}, nil which genuinely means "there was no errors, and the authoritative DNS server returned zero records of the requested type".
IOW, I'd say it's a client's issue to present this situation to a user as an error, if desired.

[mateusz834]

I have one more concern (maybe misguided): I would not say that having zero TXT records for an existing domain is an error to begin with. In the OP's case I would expect net.LookupTXT to return nil, nil or []string{}, nil which genuinely means "there was no errors, and the authoritative DNS server returned zero records of the requested type".
IOW, I'd say it's a client's issue to present this situation to a user as an error, if desired.

I don't think that this kind of change is backwards compatible at this point.

[Skeeve]

I would first make sure what @Skeeve's original wish regarding the stated problem was. If it is purely about how errors of such class look when printed, — that is one thing; if, instead, they would like to be able to programmatically differentiate between NXDOMAIN and missing records of the queried class, — that's another story.

First of all I thought that "no such host" for a missing TXT record is plain wrong. That's where I came from.

So regarding your other statement:

I have one more concern (maybe misguided): I would not say that having zero TXT records for an existing domain is an error to begin with.

I fully agree. A missing TXT record is not an error. An empty slice or a nil for the slice would perfectly indicate the "missingness" of the record

if, instead, they would like to be able to programmatically differentiate between NXDOMAIN and missing records of the queried class, — that's another story.

That would be a nice-to have. So being able to drill down without going to another module which allows "bare-bones" DNS access would be very helpful, at least to me.

I don't think that this kind of change is backwards compatible at this point.

That's for sure. Maybe providing another function for that purpose would help here?

[mateusz834]

I have one more concern (maybe misguided): I would not say that having zero TXT records for an existing domain is an error to begin with.
That would be a nice-to have. So being able to drill down without going to another module which allows "bare-bones" DNS access would be very helpful, at least to me.

This can be implemented, but i would block this on #63116, so that we don't pollute the DNSError with more fields (we already have IsNotFound). Then we can define a global errors:

var (
    ErrNoSuchDomain = errors.New("no such domain") // DNS NXDOMAIN
    ErrNoRecordsFound = errors.New("no records found") // DNS NODATA
)

But then question remains on how to handle /etc/hosts (LookupHost) (is this NXDOMAIN/NODATA or something else?).

Personally I think that if you need this kind of access you should use a DNS library directly like miekg/dns.