search

mhale / smtpd

An SMTP server package written in Go, in the style of the built-in HTTP server.
The Unlicense
397 stars 92 forks source link

Mail should only queue once handler returns without error #17

Closed bhorvath closed 3 years ago

bhorvath commented 4 years ago

The purpose of this PR is to allow messages to be processed safely before the client receives a '250 Ok' response. If the mail handler returns an error then a 4xx code is sent to the client instead, at which point it is up to the client to try again later. Without this, a message is lost if the mail handler fails to process the message. For example, if the mail handler is responsible for persisting messages, but fails, then the message wasn't actually queued and the message is lost forever.

I've also renamed Handler to MailHandler for consistency with AuthHandler (HandlerRcpt should ideally be renamed as well) and I've added in tests for the mail handler.

mhale commented 4 years ago

Thanks for the PR.

The renaming of types is going to break existing code (which I'd like to avoid), and calling it MailHandler means (under the current naming scheme) the MAIL command couldn't have an obviously named handler in the future because that name is already taken.

It might be worth making the handlers more generic. What do you think of adding an AddCommandHandler(command, func) (or similar) function?

bhorvath commented 3 years ago

Sorry for the very late follow up. I've removed the handler name changes and retained the change to error handling and test coverage. Let's discuss handler naming in a separate issue.

prologic commented 3 years ago

Can we merge this?

mhale commented 3 years ago

Thank you for the PR. I've merged it.