Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support NOOP #15

Open
cryptix opened this issue Dec 10, 2017 · 10 comments
Open

Support NOOP #15

cryptix opened this issue Dec 10, 2017 · 10 comments

Comments

@cryptix
Copy link

cryptix commented Dec 10, 2017

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
Copy link

ivy commented Dec 10, 2017

@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
Copy link

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

@pedromorgan
Copy link

@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
Copy link

@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
Copy link
Author

cryptix commented Dec 11, 2017

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
Copy link

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.

@cryptix
Copy link
Author

cryptix commented Dec 20, 2017

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
Copy link

ivy commented Dec 21, 2017

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
Copy link

ivy commented Aug 20, 2018

@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.

@cryptix
Copy link
Author

cryptix commented Aug 21, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants