Return the reply from SMTP server when sending single message #92
Conversation
Return the reply because it may contain useful information for logging and tracking. In particular, Amazon SES insists on assigning a new Message ID rather than trusting the one we send them, and they return the assigned Message ID in the reply, after "250 Ok". Only do this when `smtp-send` is called with a single message. (This is the common use case, which is documented and supported by postal.core.) If someone manages to call `smtp-send` with multiple messages, then return a response without the server replies, as before. (The alternative, which is to complicate the response from `smtp-send` to incorporate multiple replies while remaining backwards compatible, does not seem worth it---especially because sending multiple messages at a time is not something you should do if you care to know which ones succeeded.)
src/postal/smtp.clj
Outdated
|
|
||
| (defn- send-multiple [transport jmsgs] | ||
| (doseq [^javax.mail.Message jmsg jmsgs] | ||
| (.sendMessage transport jmsg (.getAllRecipients jmsg))) |
There was a problem hiding this comment.
Ah, yes, that's be fine. I'll change that.
|
This is great, thanks @ljosa! Left a question but otherwise looks good. |
| Phil Hagelberg | ||
| Roman Flammer | ||
| Sam Ritchie | ||
| Stephen Nelson | ||
|
|
There was a problem hiding this comment.
Note you removed all the newlines from these (or, likely your editor did).
There was a problem hiding this comment.
Oops. Yes, I have configured emacs to remove trailng spaces. Sorry for not reviewing the commit more thoroughly.
| (defn- send-multiple [transport jmsgs] | ||
| (doseq [^javax.mail.Message jmsg jmsgs] | ||
| (send-single transport jmsg)) | ||
| {:code 0 :error :SUCCESS :message "messages sent"}) |
There was a problem hiding this comment.
Also, this single return value will mask all the return values from the send-single invocations, which could have an error. Likely want to return a vec here that the user can scan through.
There was a problem hiding this comment.
I believe sendMessage will throw an exception if any of the messages fail. For that reason, the API for sending multiple messages is problematic. (If something fails, it's hard for the caller to figure out which messages succeeded.) But it's fine for callers that don't care to track that anyway.
I tried to not change the interface at all, since you might have users that depend on it. I'm not sure what policy you have for breaking changes.
drewr
force-pushed
the
master
branch
4 times, most recently
from
b199c5a to
382cb0e
Compare
Return the reply because it may contain useful information for logging
and tracking. In particular, Amazon SES insists on assigning a new
Message ID rather than trusting the one we send them, and they return
the assigned Message ID in the reply, after "250 Ok".
Only do this when
smtp-sendis called with a single message. (Thisis the common use case, which is documented and supported by
postal.core.)
If someone manages to call
smtp-sendwith multiple messages, thenreturn a response without the server replies, as before. (The
alternative, which is to complicate the response from
smtp-sendtoincorporate multiple replies while remaining backwards compatible,
does not seem worth it---especially because sending multiple messages
at a time is not something you should do if you care to know which
ones succeeded.)