/cc @mikioh

I see there are 6 uses of unexposed errNoSuchHost in the Go codebase.
See: https://github.com/golang/go/search?q=errNoSuchHost&unscoped_q=errNoSuchHost.

Including the test file where errNoSuchHost is checked with string comparison. See:

IMHO exposing this error variable and refactoring the codebase would be enough.

Hi, I'd like to take this up if it is free

I've missed that errNoSuchHost is always wrapped into DNSError before it is returned.
Could we add a variable IsNoSuchHost to DNSError struct. Then we could assert returned error to DNSError and check if IsNoSuchHost==true. See https://play.golang.org/p/WyhepLJg2WI

@maksadbek I'm assuming you are taking this up?

Hi @affanv14
Yes, I'm ready to send a pr if it's ok to use the option I described above.

I'm a new contributor myself, I mistook you for a member, I think you may have done the same! Anyway, I'll leave this up to you. Good luck on the PR!

You could also implement „methods“ on top of DNSError, then you don’t need an extra variable and expose the data required.

@szuecs Do you mean a method that checks prefix of the err text ?

Instead of storing it in a variable, you could do this yes.
It was just an idea, no need to do this, but maybe it's not so bad. :)

I'd like to take this up. Shall I start work on a CL, if nobody's doing it?

Go for it.

@gabber12 First tell us exactly how you plan to change the API. Don't send in new API via a CL. Discuss it first. Thanks.

I am inclined to add a bool var (IsNoSuchHost)in DNSError itself(instead of doing string comparison).

type DNSError struct {
	Err           string // description of the error
	Name          string // name looked for
	Server        string // server used
	IsTimeout     bool   // if true, timed out; not all timeouts set this
	IsTemporary   bool   // if true, error is temporary; not all errors set this
	IsNoSuchHost bool   // if true, host could not be found
}

The idea would be to make IsNoSuchHost true wherever errNoSuchHost is being wrapped in DnsError.

There is a todo too, to do this in dnsclient_unix.go

@ianlancetaylor

Thanks. I would like to hear from @jba and @neild how this might interact with the error values proposal being considered for 1.13, and whether there is a different approach that might be a better fit with that.

If ErrNoSuchHost is exported, in 1.13 you will be able to test to see if a *DNSError is a no-such-host error with errors.Is(err, net.ErrNoSuchHost). No need to add an additional field to the DNSError struct.

This requires adding an Unwrap method to DNSError, which I'm planning on doing soon.

DNSError doesn't currently wrap the error itself, just its message. Is there a concern with exposing the underlying error?

@jba Oh, sorry; an Err field is so conventionally an error that it didn't register for me that this one is a string.

In this case, adding another boolean is probably the way to go since it's consistent with the existing IsTimeout and IsTemporary fields.

Shall we go with

• Adding a boolean flag in DNSError
• Or, Make err field in DNSError an Error instead of string

Personally i see similar tradeoffs in both approaches. And it really depends on the Unwrap method's consistency across std lib

For me it's quite unclear what is meant by Temporary() and Timeout(), which one are retrieable, so while starting a connection?
IMO it would be much better to have errors as types and let the user decide what to do and coument which types are raised for what cases.

@szuecs I can understand that Timeout can be an error type, but the Temporary() api will still have to be a method, since different errors may be temporary or permanent.
Leaving on the user to maintain which errors are temporary might not work out.

But overall exposing errors to user seems like a good approach to me too.

@jba @neild would love to hear your take on this.

@gabber12 You can't change the type of the DNSError.Err field without violating compatibility. If we were starting from scratch I'd recommend making it an error rather than a string, but it's too late now.

@neild Make sense, since Err is an exported field.
I would agree with going on with the original approach of adding a boolean flag in DNSError.

Shall i go ahead with the same?

Change https://golang.org/cl/168597 mentions this issue: net: add NoSuchHostFound to DNSError

Thanks for fixing the issue!

@gabber12 does not make it more sense to have an enum style type with iota. that you have instead of the bool flags and check the type instead of the bool flags?