-
Notifications
You must be signed in to change notification settings - Fork 53
Description
In the Keyshare task, emails are being sent to new users and users who's account are reaching the account deletion mark after not having logged in for a while.
Logging shows that the task tries to send out an email to a particular domain on every run.
However, the DNS-lookup for an MX-record fails and no email is sent, upon which the next runs it will try again and again and again.
I've checked and domain cannot be found by either local nor online DNS-lookup tools.
level=error msg="No active network connection" error="lookup _<non-existing-domain>_.nl on xx.xx.xx.xx:53: server misbehaving"
For a single entry, this isn't too bad. Howevery if we stack up multiple of this issues, the task might eventually only try to send emails which can't be delivered, queueing the 'valid' emails to be sent.
The origin of the issue lies in the verification of an MX-record, which is not being found and the secondary check in an IP lookup. (VerifyMXRecord func, see code)
Three things I've found which could be checked/improved:
- The DNSError contains an
IsNotFoundflag which is not checked. I'm not sure if this flag is used when only doing an MX lookup (or if we should rely upon it before we check for A / AAAA records). - The
IsTemporaryflag is set by an internal library, using a call to a deprecatedTemporary()func. I would consider removing this flag. - In case of a 'timeout', we might want to try an IP-lookup first and only there check if it is truely a connection issue, because it almost looks as-if the timeout isn't a true timeout.
records, err := net.LookupMX(host)
if err != nil || len(records) == 0 {
if derr, ok := err.(*net.DNSError); ok && (derr.IsTemporary || derr.IsTimeout) {
// When DNS is not resolving or there is no active network connection
server.Logger.WithField("error", err).Error("No active network connection")
return ErrNoNetwork
}
// Check if there is a valid A or AAAA record which is used as fallback by mailservers
// when there are no MX records present
if records, err := net.LookupIP(host); err != nil || len(records) == 0 {
return ErrInvalidEmailDomain
}
}