sofn-xyz / mailing

Build, test, send emails with React
https://www.mailing.run
MIT License
3.6k stars 74 forks source link

Return a mocked status when sending emails in test/preview mode. #473

Open danielthedifficult opened 1 year ago

danielthedifficult commented 1 year ago

Is your feature request related to a problem? Please describe. I'm wanting to check that an email was successfully sent (or not) before deleting an item from our notification queue.

In prod/regular sending mode, it returns the .json() of the underlying nodemailer result on line 68:

https://github.com/sofn-xyz/mailing/blob/cea4bdb4644792f9fda8af3dc6de417ed727e0fe/packages/cli/src/pages/api/sendMail.ts#L60-L76

Unfortunately, in test/preview mode, the sendMail function doesn't return anything. Maybe this was a simple oversight? Could it be as simple as just adding a return in front of lines 46 and 50? https://github.com/sofn-xyz/mailing/blob/cea4bdb4644792f9fda8af3dc6de417ed727e0fe/packages/cli/src/pages/api/previews/send.ts#L40-L53

Or maybe I'm confused and these are actually the relevant lines: https://github.com/sofn-xyz/mailing/blob/cea4bdb4644792f9fda8af3dc6de417ed727e0fe/packages/cli/src/preview/controllers/intercepts.tsx#L19-L32

UPDATE:

I just tried, on a hunch, some .then/.catch syntax and it works:

    await sendMail({
      to: someEmail,
      component: <MyEmailTemplate />,
    })
      .then(() => {console.log('RESOLVED')})
      .catch(() => console.log('FAILED'));

logs "RESOLVED" when testing (even with no to: email address) and "FAILED" if I don't provide a component prop.

Maybe that's GEFN?

Describe the solution you'd like I think returning a mocked status that closely resembles what nodemailer would give, i.e. a successful promise with perhaps an auto-generated message ID, 200 status code, etc., would be best.

psugihara commented 1 year ago

Heya, sorry for slowness on the response. Deprioritized when I saw your workaround then forgot 🙃

I believe this is the block returning undefined: https://github.com/sofn-xyz/mailing/blob/cea4bdb4644792f9fda8af3dc6de417ed727e0fe/packages/core/src/index.ts#L151-L160

Your workaround seems good if that works for you. The difficulty with this feature would be that different transports will have different response types. I'm open to just making a generic response (e.g. {} rather than undefined) but I worry that going too much more complicated would result in confusion when looking at the sendMail response in dev vs prod.

I'd opt to close if the workaround is good enough but lmk.

danielthedifficult commented 1 year ago

I think returning an empty object is a good start. I am confused about how standard a "transport" is, and whether or not they all return the SMTP stats codes, etc.

But, again, with this workaround, a bit of documentation, and returning an empty object that's probably enough.

psugihara commented 1 year ago

Found the doc on transport return types here: https://nodemailer.com/plugins/create/#transports

Looks like the common return type includes envelope and messageId. For the 3 built-in nodemailer transports they look like this:

envelope – is an envelope object {from:‘address’, to:[‘address’]}
messageId – is the Message-ID header value. This value is derived from the response of SES API, so it differs from the Message-ID values used in logging.

I'd be open to just use that as a spec for a dummy return value.

Of course, there are no promises that 3rd party nodemailer transports (e.g. the mailgun one) conform to this.

danielthedifficult commented 1 year ago

That's perfect, exactly what I was hoping. Nice find.