axllent / mailpit

An email and SMTP testing tool with API for developers
https://mailpit.axllent.org
MIT License
6.17k stars 153 forks source link

Feature Request: Log failed messages somewhere #347

Closed knpwrs closed 3 months ago

knpwrs commented 3 months ago

I was attempting to send some emails to Mailpit and it turned out they were malformed. Mailpit was having issues parsing the headers. I would see these errors in the log output, but no information was available in the web UI. It would be great if the raw email content could be stored somewhere for examination in the web UI.

axllent commented 3 months ago

This is an interesting request @knpwrs. What was malformed in your messages exactly? Surely you were getting an SMTP error (rejection) while sending the malformed message, and if so, what error was reported there? What error message was logged in Mailpit's log? I ask because I am trying to understand the context of the error you experienced, and what was the cause and result.

I am reluctant to add a this feature for a couple of reasons:

  1. Handling message rejection (for whatever reason) should be handled via the SMTP client/application. When Mailpit fails it should return the error. Mailpit is designed to display the captured data, but you're asking for it to also store the data that was not captured. This is somewhat out of scope for the purpose of the application.
  2. Errors can be caused by many things and don't necessarily occur because of just bad message headers. There are plenty of invalid SMTP combinations such as bad username/password combinations, or too many recipients, wrong TLS protocol etc. Your proposal to store the raw (bad) message data creates a lot of technical overhead to provide insight into a small subsection of potential errors. Again, without knowing the reason your messages were malformed and rejected it's difficult for me to guess.

One potential option could be to display a temporary error notification in Mailpit's UI, displayed to any connected browser (similar to when a message is received). The issue with this approach is that it's more of a "hey a message just got rejected for XYZ", but then again, if someone has Mailpit exposed to the internet and a script kiddy is hammering away trying to authenticate (brute force), then this would flood the web UI with error messages too, so it's not ideal either. This also requires the user to be connected and see the error in a few seconds it is displayed.

I'm keen to hear more about your error though.

knpwrs commented 3 months ago

I did indeed get an SMTP rejection error, and I saw that there was a malformed header in the container logs for mailpit. The thing that was tricky for me was that I was using a go server, a Faktory queue with a go worker calling out to net/smtp, and mailpit. This is my first time using both Go and Faktory, and so it took me a little bit longer to find the bug than if I had been using a more familiar stack.

So this is really just a “would have been neat to see” idea. I was watching the mailpit UI and didn’t see anything, and so I assumed the message never reached mailpit, something along the way must have errored out. I started tracing from the beginning and came to find that it did indeed reach mailpit, but the message was rejected.

The issue was that I had an email roughly like this:

From: sender@example.com
To: recipient@example.com
Subject: Multipart Email Example
multipart/alternative; boundary="boundary-string"

--your-boundary
Content-Type: text/plain; charset="utf-8"

Hello, World!

--boundary-string
Content-Type: text/html; charset="utf-8"

<h1>Hello, World!</h1>

--boundary-string

As you can see, there is a value for Content-Type, but Content-Type itself is missing. This is quite the error on my part!

So really any way of surfacing these errors to the mailpit UI would be nice to have since I was watching the mailpit UI anyway. It doesn’t even have to be in the message list. Maybe a separate tab that’s like an audit log, or even like you suggested, some sort of temporal error message for connected clients.

axllent commented 3 months ago

Thanks for the information, this is both useful for me to try understand the situation, and so make a more informed decision. Whilst storing rejected messages together with the reason sounds ideal, I don't think it is a feasible option as it introduces a rather large level of complexity because those messages need to also be stored somewhere, and these are likely only a fraction of the potential errors themselves. I believe that in your case a simple "alert" in the UI that a message had been rejected (with whatever reason you got from SMTP) would have been sufficient to help you diagnose the issue.

The easiest solution here would be to fire-and-forget a warning message to all connected browsers, meaning no need for any storage or special UI elements (other than what is already there), however that is possibly a bit too short-sighted in the scheme of things. This approach would require very few moving parts and wouldn't take too much to implement.

The other potential solution would be to both display a temporary notification (just like when a new message is received) as well as "store" a certain number of error messages (eg: up to 100 or something). The UI would need some sort of "error logs" section where one could then view these. This is more complex than the easy solution mentioned above, but would cater for when a browser isn't connected (or a user isn't watching) as well as, within reason, historical errors.

I need to give this a lot more thought as to how I approach this, and what the best way forward is. If you have any more thoughts then please feel free to share them in the meantime.

axllent commented 3 months ago

@knpwrs I have added temporary (5 second) web UI error notifications in v1.20.2 after deciding that a whole UI logging system is just not feasible. There are so many different types of potential errors when it comes to the Mailpit ecosystem (it's a lot more than just SMTP), which makes a UI logging system a very complicated "nice to have" task. Client errors should always be handled properly client-side anyway.

Hopefully this helps you identify similar issues though, and I hope you understand the reasoning not adding the logging system to the UI?

knpwrs commented 3 months ago

That's very good, thank you @axllent! The only thing I'd suggest is maybe a persistent dismissible notification, but a five-second notification probably could have helped a lot.

Thank you again!