go-mail/mail

Support NOOP

Opened this issue · 10 comments

cryptix commented

Hi!

I managed to get the NOOP method into net/smtp (golang/go#22321) and it looks like it will be part of Go v1.10*.

I would like to use it to improve connection handling here. My current thinking is to use the NOOP periodically and/or before trying to send a mail and re-dial a number of times if the connection broke.

How does this sound?

Another open question would be how to make this backwards compat. I imagine a checkConnection() bool or something to hide the implementation detail so that builds pre-1.10 don't break.

(I pushed a very rough first draft to my useNOOP branch. You can compare next..useNOOP here. It's not functional yet but I wanted to see how it could fit into the current design.)

ivy commented

@cryptix Thanks for opening this issue. If you're aware of other libraries that make use of NOOP in this way, I'd love some references.

Also, this may be part of a larger desire that comes up frequently upstream: providing access to the underlying *smtp.Client.

pedromorgan commented

I ensure my mailservers dont fo that NOOP shite.. stops spammers

pedromorgan commented

@ivy your a spammer in your reality,

U need to dive and understand the mailserver itself to become the
"the master of the art"..

But I'm spammer also, but for "legal reasons"..

pedromorgan commented

@ivy one of your comments before was to test it out on a real smtp..

So am making one expecially for u @ivy and rpi.. for everyone..

its a whole config in itSelf on a shelf..

U up for that @ivy
U can have 2 of them..
one runnign postfix
the other sendmail
its basically what i am doing here..

cryptix commented

sorry @pedromorgan I didn't understand any of your points. Do we really need to expand the drama (#16) here?

To get back on topic: How does noop help spammers? I don't see the relation. Isn't (black)listing based on source ips and addresses?

theckman commented

It might be good to think about how consumers of this API can turn this functionality on instead of making it a default action. Some SMTP servers do really silly things, even outside of spec, so it's often better to default to the simplest implementation and allow consumers to turn on more advance features.

  • 👍1
cryptix commented

It might be good to think about how consumers of this API can turn this functionality on instead of making it a default action.

yes. I also had this thought. I'd also wish to make it opt-in but still have automatic reconnect, maybe hidden inside the checkConnection() I alluded to above. Not sure how finegrained we want this.

ivy commented

I guess my biggest concern is how we would implement this behavior without getting in the way. Would it be enough to add a Conn() method to the Sender interface and let developers implement this behavior themselves? That would allow transports to expose their underlying connection for special handling:

client, ok := sender.Conn().(*smtp.Client)
if !ok {
	// [some error handling logic...]
}

// A naive implementation as an example:
go func() {
	for _, t := range time.Tick(10 * time.Second) {
		client.Noop()
	}
}()

I imagine this could also work for other transports like sendmail and REST APIs (e.g., Mandrill, SES, Mailgun, etc.). Perhaps we could even provide convenience methods to make this easier.

Edit: Fixed a typo in my code example to use client.Noop(), not client.Reset().

ivy commented

@cryptix After the discussion in go-gomail#123 I realize I must have totally missed your point when you first opened this issue. 😅

Correct me if I'm wrong, but what you're hoping to do is send a NOOP periodically to maintain an open/idle connection, right? I still don't think we can do this within the library without causing issues for a lot of users. From looking at some of the reported errors, there doesn't seem to be a consistent error returned by mailservers and the behavior is totally arbitrary:

  • Gmail1 - 451 4.4.2 Timeout - closing connection. xxxxxxxxxxxxxxx.33 - gsmtp
  • Name.com2 - 554 Too many nonmail commands
  • IIS3 - 421 Too many errors on this connection---closing

The case for IIS is especially troubling in that it sounds like you'll be disconnected after 10 NOOPs have occurred in a session, regardless of whether or not they all happened in sequence.

I'd love to know what you think about this. Right now, I'm leaning more toward exposing the underlying SMTP client.

  • 👍1
cryptix commented

Correct me if I'm wrong, but what you're hoping to do is send a NOOP periodically to maintain an open/idle connection, right?

My first idea was using one NOOP before trying to send a mail and redial if that gave an error but I didn't know about that IIS behavior, which is troubling for that approach.

I'd love to see some kind of delivery pipeline / retry behavior but can very much understand if you see it as out of scope for this package.