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

Feature request: API endpoint for sending #278

Closed
ryan0x44 opened this issue Apr 17, 2024 · 10 comments
Closed

Feature request: API endpoint for sending #278

ryan0x44 opened this issue Apr 17, 2024 · 10 comments

Comments

@ryan0x44
Copy link

ryan0x44 commented Apr 17, 2024

When building apps on frameworks like Next.js and Remix.run for platforms like Vercel and Cloudflare Pages, people tend to use services which offer a HTTP integration like Resend, SendGrid, Postmark, etc. as SMTP support is tricky (e.g. running nodemailer doesn't necessarily work).

Mailpit has a great HTTP API, however there is no endpoint for sending email. Adding one would make it easy to send test emails from these platforms.

As a bonus, offering API-compatibility with the most popular services like SendGrid, Resend, Postmark, Mailgun, AWS SES, etc. would mean that in development users could re-use the same code.

@axllent
Copy link
Owner

axllent commented Apr 18, 2024

Firstly, thank you for your contribution @ryan0x44. This has been discussed in #167 where I actually rejected the idea which also proposed a mock-api of these various services. This was primarily because these services all offered different implementations & features for their APIs, so there was no one-shoe fits all, and I just did not intend on trying to maintain a separate set of ever-changing APIs and have to reverse-engineer each one.

Please note that this is just to open up a conversation to try understand the problem better, and not to reject your idea....

You say "offering API-compatibility with the most popular services like", but is it? I'm asking purely on the assumption that you have more experience than I do with these services. Looking at a couple of them now again, I see:

Mailpit (as proposed by your PR)

{
    "from": "[email protected]",
    "to": [
        "[email protected]"
    ],
    "subject": "hello",
    "bodyHTML": "<html><body>Hello</body></html>",
    "bodyText": "Hello"
}

Sendgrid

{
    "personalizations": [
        {
            "to": [
                {
                    "email": "[email protected]",
                    "name": "John Doe"
                }
            ],
            "subject": "Hello, World!"
        }
    ],
    "content": [
        {
            "type": "text/plain",
            "value": "Heya!"
        }
    ],
    "from": {
        "email": "[email protected]",
        "name": "Sam Smith"
    },
    "reply_to": {
        "email": "[email protected]",
        "name": "Sam Smith"
    }
}

Resend

{
    "from": "Acme <[email protected]>",
    "to": [
        "[email protected]"
    ],
    "subject": "hello world",
    "html":"<html><body>Hello</body></html>",
    "text": "it works!",
    "headers": {
        "X-Entity-Ref-ID": "123"
    }
}
  • Mailgun uses multipart/form-data
  • Postmark has different APIs for sending single and batch emails
  • etc etc

My point is they are all very different, and all of these support far more than just a text & html part (eg: attachments, headers, cc, bcc, reply-to etc).

So to come back to my questions:

  1. How does this offer "API-compatibility" with any of those services? Do you just mean that Mailpit would also offer a HTTP API to send (or rather in this case receive) email - in which case we really cannot say "API-compatibility"?
  2. Assuming it is just another HTTP API, wouldn't this mean that an application would need a separate set of "sending instructions" to test with, ie: a separate library etc? How would you expect developers using similar frameworks be expected to use the Mailpit API to send messages?
  3. I believe that any HTTP API to do this, no matter how simple, requires a lot more flexibility and features (eg: Cc, Bcc, headers and attachments). I'm not proposing you change your PR yet as I think there is probably a better approach to message construction (assuming I do add a HTTP sending API) to build the message (as your implementation is fairly hard-coded - ie: requires both a HTML and text body). What you have provided here does provide a foundation to work from.

Keen on your thoughts & feedback.

@ryan0x44
Copy link
Author

ryan0x44 commented Apr 25, 2024

Hey thanks @axllent for the thoughtful response!

On reflection I think you're right that it wouldn't make sense to try and support all of the different APIs necessarily. It would be a huge amount of work to build and maintain. I think this is why I suggested it might be a bonus, as opposed to a key part of this request. Sorry I missed #167 during my search btw! But good to know others have an interest in some kind of HTTP API.

I do see it as crucial functionality to offer some kind of HTTP API for sending messages. In my context I basically can't use mailpit without it (or running some kind of proxy).

I think my PR has a couple of improvements that could be made (e.g. I don't think the correct headers are being set), but it's working well enough that I can see the subject and HTML/Text body of the emails in development now. Keen to get some eyes and feedback on that, and would be happy to try and get it well-formed enough for a merge! (assuming you're ok with adding that basic functionality?)

@axllent
Copy link
Owner

axllent commented Apr 25, 2024

Thanks @ryan0x44. I do understand now (better) the merit of such a feature, and have been giving this a fair bit of thought in the last week. I guess where my confusion lies is in the requirement, if any, to align the Mailpit HTTP API with any of the others (eg: those previously mentioned), or whether, from a developer's perspective, it actually doesn't matter?

What I am hesitant about is adding a HTTP API (regardless how), and users not being able to use it because it doesn't align with their chosen "SMTP" provider's HTTP API (ie: their app's current functionality). If it is common for developers to just switch out the library (or rather have a testing fallback library) when testing, then this is a non-issue - your feedback is welcome as I have no idea!

As part of my thought process over the last week, I thought it may be possible to eventually create "simplified versions" of some of the more common APIs, "mapping" the posted data to the supported Mailpit API. This way at least they could theoretically send using their existing library (assuming it allowed the API URL to be changed to Mailpit's), though maybe not all features are supported. This would be a secondary feature once I knew the Mailpit API was stable.

So the real question I have is, in your opinion, is having a different API to an issue for developers in your situation? If not, is it normal for JS developers to have separate library packaged for testing, and if so, how would a developer go about switching between the two? I am just trying to get a better understanding.

As a secondary question, if/when Mailpit would get this functionality, would you be willing to create and maintain a JS library for this, or do you think it's not necessary? Again, I don't know what JS developers prefer in these situations.

As a final thought, I would likely be looking at implementing it with the structure of something like Resend as this seems flexible enough, although using arrays more for addresses (including separate name & email fields), attachments etc - and thus the email generation (in Mailpit) would be very different to what you created. Don't worry, I am not asking (or wanting) you to modify what you have, I will do that work as it'll need to fit into the bigger picture of what I have potentially planned. Please leave your PR as-is for now as I can use that for reference.

@axllent axllent added the enhancement New feature or request label Apr 27, 2024
@axllent
Copy link
Owner

axllent commented Apr 30, 2024

This is what I have come up with so far in developmen:

{
  "From": {
    "Email": "[email protected]",
    "Name": "John Doe"
  },
  "To": [
    {
      "Email": "[email protected]",
      "Name": "Jane Doe"
    }
  ],
  "Cc": [
    {
      "Email": "[email protected]",
      "Name": "Manager"
    }
  ],
  "Bcc": [
    {
      "Email": "[email protected]",
      "Name": "Jack Black"
    }
  ],
  "ReplyTo": [
    {
      "Email": "[email protected]",
      "Name": "Secretary"
    }
  ],
  "Tags": [
    "Tag 1",
    "Tag 2"
  ],
  "Subject": "Mailpit API subject",
  "Headers": {
    "X-IP": "1.2.3.4"
  },
  "Text": "This is the text body",
  "HTML": "<p>This is the <u><b>HTML</b></u> body</p>",
  "Attachments": [
    {
      "Content": "VGhpcyBpcyBhIHBsYWluIHRleHQgYXRhY2htZW50Lg==",
      "Filename": "AttachedFile.txt"
    }
  ]
}

The only fields which are mandatory are the From.Email and To.Email, the rest are optional. I think this covers most user-cases - what do you think @ryan0x44 ?

@ryan0x44
Copy link
Author

ryan0x44 commented May 1, 2024

So the real question I have is, in your opinion, is having a different API to an issue for developers in your situation? If not, is it normal for JS developers to have separate library packaged for testing, and if so, how would a developer go about switching between the two? I am just trying to get a better understanding.

As a secondary question, if/when Mailpit would get this functionality, would you be willing to create and maintain a JS library for this, or do you think it's not necessary? Again, I don't know what JS developers prefer in these situations.

I don't think it's an issue - it's such a trivial amount of code to support a second mail provider (maybe 30 lines, largely just for building the payload). I think for the time-being just supporting the proposed API (from your last comment) makes sense, and at a later date you could always support additional formats for compatibility with things like Resend by adding a query param or similar.

I really like your API proposal - I've updated the PR to align to that, and also resolved issues where the payload didn't contain the right headers for From/To/etc. So now in my testing it appears to be working exactly how I'd hoped!

Let me know if you have any other feedback on PR #279 , would be fantastic to get this merged :)

@axllent
Copy link
Owner

axllent commented May 1, 2024

@ryan0x44 Thanks for the feedback. As I mentioned before (but maybe wasn't clear on), I am already actually implementing a different version of this functionality in another local branch, so I don't actually need your PR (to merge). The way I'm doing it is somewhat more flexible, eg: there is no forced HTML multi part, support for attachments, etc, and the way I'm doing it ties into a few other things too.

I do appreciate your effort though! Please leave your PR as-is as I am referencing it to see if I don't leave out anything. My plan is to have something merged and released within the next few days (definitely by the end of the weekend).

@ryan0x44
Copy link
Author

ryan0x44 commented May 1, 2024

@axllent are you saying you're not open to outside contributions?

FWIW, my PR has conditionals around those fields so they're already optional. I've also just updated it to support attachments, customers headers, and tags - I think that's everything. More than happy to make any other changes you want to see. It would be nice to be able to contribute :)

@axllent
Copy link
Owner

axllent commented May 2, 2024

Yes of course, I'm absolutely open to outside contributions @ryan0x44 - but contributions does not necessarily equate to an accepted PR. There is little point to me accepting any PR when it's not going to do exactly what I need it to do, and so I end up rewriting much of it. I thought I was quite clear that I was intending to implement this feature myself in a manner I saw fit for Mailpit and future functionality. I have already done most of the coding and API docs, I just need to add some relevant tests, potentially documentation on the website, and of course a lot more testing.

The fact of the matter is you have already contributed a lot here (for which I am grateful), and yes, this feature will make its way into Mailpit in the near future, but it won't be this PR. There are some fundamental flaws in your templated approach that just won't work to provide valid RFC email structure, and there is a far easier manner to build the email (which is just one of the different approaches I have taken), not to mention the handler etc. It's not that your approach won't work for your use-case, it's just that it won't work for everyone.

@axllent
Copy link
Owner

axllent commented May 3, 2024

This new feature has been released in v1.18.0.

Hopefully you'll see the very different approach I took to your implementation, and clears up why it made no sense to to merge your PR? I did credit you for your contribution in the release notes.

I look forward to your feedback and testing!

@axllent axllent removed the enhancement New feature or request label May 3, 2024
@ryan0x44
Copy link
Author

ryan0x44 commented May 6, 2024

Great to see this change released! Can confirm it works as expected (once I accounted for the slightly different URL path).

I don't see a significant difference on the implementation, but your solution works and I appreciate the credit in the release notes. Thanks @axllent !

@ryan0x44 ryan0x44 closed this as completed May 6, 2024
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

No branches or pull requests

2 participants