Log sent emails and error out if non-SMTP is used in production #4627
Conversation
Before this commit, custom metadata was stored as part of a request extension, which required the mutable request object to be passed to every place which needed to log something. This commit changes the implementation to use a thread local that is cleared before a request is processed. This removes the dependency on the mutable request object. Note that when migrating the crates.io codebase to async the thread local will need to be replaced with an async equivalent.
|
Note that I don't know whether Mailgun will override the message ID with its own or not. I tested the change locally with a different mail server I have access to, but I don't know how Mailgun behaves in regards to this. |
Turbo87
added
A-backend ⚙️
C-internal 🔧
| server_error("Failed to send the email") | ||
| })?; | ||
|
|
||
| add_custom_metadata("email_id", message_id); |
There was a problem hiding this comment.
is there a reason for adding the email_id only after the email error was handled? wouldn't it be better to set it as soon as we generated it?
There was a problem hiding this comment.
There is no use for the Message-ID if Mailgun returned an error: in that case, the mail wouldn't have been sent and wouldn't show up in the Mailgun console, so having the ID offers no practical benefit.
|
@bors r=Turbo87 |
|
📌 Commit b7a75f5 has been approved by |
|
☀️ Test successful - checks-actions |
3 participants
This PR does three things:
&mut dyn RequestExtto the email methods (as that's not really clean and caused borrow checker issues).This PR is best reviewed commit-by-commit.
Fixes #4596
Fixes #4597