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

Add support for retrieving last server reply from sendMail() #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ThomasTJdev
Copy link

Problem

The current implementation only verifies that the last reply is 250. However, it does not provide access to the actual reply message. Services like AWS SES include essential data in the last reply, such as 250 ok <messageId>, which is critical for tracking emails.

Solution

This PR modifies the existing sendMail() procedure to return a Future[string] instead of discarding the server's response. The new version of sendMail() has been renamed to sendMailGetReply() to reflect its behavior. A new sendMail() wrapper was introduced, which calls sendMailGetReply() but discards the result, ensuring full backward compatibility.

Additionally, the final usage of the checkReply procedure within sendMailGetReply() has been replaced with inline code, as checkReply does not return any value.

Notes

  1. sendMail(): Ideally, the original sendMail() would be marked as .discardable. to allow users to optionally retrieve the reply. However, the asynchronous multisync blocks this approach.
  2. checkReply(): It would also be preferable to make checkReply .discardable. and directly use its return value for simplicity. However that requires (1) first.

Example

client.sendMail(smtpFrom, @[recipient], message)

let reply = client.sendMailGetReply(smtpFrom, @[recipient], message)
echo reply # 250 ok 01220594310df52a-b6544dcb-a4e1-49b3-9a6a-7aaf1a7d31de-000000

Created a new proc, sendMailGetReply(), which returns the server message. The old sendMail() proc just calls this and discards the result.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant