sofn-xyz / mailing

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

forceSend fails, tries to connect to 587 #474

Open danielthedifficult opened 1 year ago

danielthedifficult commented 1 year ago

Describe the bug

In development, when trying to use the FORCE_SEND feature within the email preview, it fails with a 500 error on the /api/sendMail request.

I suspect this has to do with my recently implementing a mail server on port 587, but your /api/sendMail API is listening on either an http/https or standard SMTP port 25.

I don't believe it's my transport config because when I manually insert dangerouslyForceDeliver: true into my buildSendMail config, it sends the email correctly.

Server logs:

mailing 500 /api/sendMail 12ms
- error unhandledRejection: Error: connect ECONNREFUSED 127.0.0.1:587
    at __node_internal_captureLargerStackTrace (node:internal/errors:478:5)
    at __node_internal_exceptionWithHostPort (node:internal/errors:656:12)
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1278:16) {
  errno: -61,
  code: 'ESOCKET',
  syscall: 'connect',
  address: '127.0.0.1',
  port: 587,
  command: 'CONN'
}

To Reproduce

Steps to reproduce the behavior:

Configure a transport using port 587 as such:

const transport = nodemailer.createTransport({
  host: process.env.EMAIL_HOST, // My mailgun URL
  port: process.env.EMAIL_PORT, // this is set to 587
  secure: process.env.EMAIL_SECURE, // this is true
  auth: {
    user: process.env.EMAIL_USER, // My mailgun username
    pass: process.env.EMAIL_PASSWORD, // My mailgun password
  },
});

transport.verify();
export const sendMail = buildSendMail({
  transport,
  defaultFrom: process.env.EMAIL_FROM,
  configPath: './mailing.config.json',
});
  1. Send an email
  2. Intercept opens
  3. Click "Force Sent"
  4. Get message above

Expected behavior

I expect it to send using my normal transport setings.

Screenshots

Screen Shot 2023-07-17 at 13 18 28

psugihara commented 1 year ago

Hm that is an unexpected error. It looks like it's trying to connect to localhost port 587 rather than your mailgun url to send the email. This fails of course because you are not running anything on 587, mailgun is.

Are you sure your env variables are set correctly for the preview server and you've tried restarting the preview server? My best guess is that the env variable is not getting set somehow. Try logging process.env.EMAIL_HOST before transport.verify and check the preview server's console just to make sure.

Otherwise there must be a bug with how mailing is ingesting your env variables.

danielthedifficult commented 1 year ago

I don't believe it's my transport config because when I manually insert dangerouslyForceDeliver: true into my buildSendMail config, it sends the email correctly.

@psugihara Happy to try logging the EMAIL_HOST but did you see the above? Without changing any env vars, etc., if I simply add that param to the same code that was triggering the preview, the email sends perfectly... so it seems like the post-intercept sending code isn't picking up the env vars.

E.g.:

  1. Try to send, preview triggers.
  2. Try to force send, errors as described above.
  3. Modify code that sends in step 1 to use dangerouslyForceDeliver.
  4. Try to send, email delivers perfectly.

Make sense?

Thanks !

psugihara commented 1 year ago

The thing I was trying to tease out is whether the env variable is set in your app process but not the preview server process.

If the env var is not being picked up, I'd recommend blowing away .mailing and restarting the preview server fresh.

But if both cases (steps 2 and 4) of trying to send are via the button then that absolutely sounds like a bug because the button sends via the mailing preview server.

danielthedifficult commented 1 year ago

No, step 4 is from within the app, with that flag set to skip the preview.

Sending from the FORCE_SEND option doesn't ever work in preview.

Logging process.env.EMAIL_HOST yields undefined.

Note: My .env is defined in .env.local, not .env. Is that at play?

danielthedifficult commented 1 year ago

Okay, further wonkiness.

  1. I was mistaken about my console logging in my previous comment. Logging EMAIL_HOST, EMAIL_FROM, and EMAIL_PORT all yield correct values.
  2. The EMAIL_PORT I am using is 465...not 587. Where is 587 coming from?
  3. I'm getting an error upon starting Mailing. The following console.log correctly logs the default host, sender and port as defined in my .env.local file in my app's console.
console.log(16, process.env.EMAIL_FROM);
export const sendMail = buildSendMail({
  transport,
  defaultFrom: process.env.EMAIL_FROM,
  configPath: './mailing.config.json',
});

export default sendMail;

^^ in my app's API libraries.

However, as soon as I start the mailing server, I get the following error:

- wait compiling /api/sendMail (client and server)...
- event compiled successfully in 1033 ms (393 modules)
- error ../node_modules/mailing-core/dist/mailing-core.cjs.dev.js (876:0) @ buildSendMail
- error Error: buildSendMail options are missing defaultFrom
    at buildSendMail (webpack-internal:///(api)/../node_modules/mailing-core/dist/mailing-core.cjs.dev.js:870:11)
    at eval (webpack-internal:///(api)/./src/moduleManifest.js:79:114)
    at Module.(api)/./src/moduleManifest.js (webpack://mailing/node_modules/@faire/mjml-react/index.js?c4c6:18:30)
    at null.__webpack_require__ (/Users/dm/Documents/prediction-buddy-v2/.mailing/.next/server/webpack-api-runtime.js:33:43)
    at eval (webpack-internal:///(api)/./src/pages/api/sendMail.ts:6:73)
    at Module.(api)/./src/pages/api/sendMail.ts (webpack://mailing/node_modules/@faire/mjml-react/index.js?c4c6:18:30)
    at null.__webpack_require__ (/Users/dm/Documents/prediction-buddy-v2/.mailing/.next/server/webpack-api-runtime.js:33:43)
    at null.__webpack_exec__ (webpack://mailing/node_modules/@faire/mjml-react/index.js?c4c6:18:30)
    at null.<anonymous> (webpack://mailing/node_modules/@faire/mjml-react/index.js?c4c6:18:30)
    at Object.<anonymous> (webpack://mailing/node_modules/@faire/mjml-react/index.js?c4c6:18:30)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._compile (/Users/dm/Documents/prediction-buddy-v2/node_modules/mailing/node_modules/esbuild-register/dist/node.js:2258:26)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.newLoader (/Users/dm/Documents/prediction-buddy-v2/node_modules/mailing/node_modules/esbuild-register/dist/node.js:2262:9)
    at Object.extensions..js (/Users/dm/Documents/prediction-buddy-v2/node_modules/mailing/node_modules/esbuild-register/dist/node.js:4807:24)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at runApi (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/next-server.ts:922:30)
    at handleApiRequest (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/next-server.ts:1671:17)
    at Object.fn (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/next-server.ts:1594:34)
    at Router.execute (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/router.ts:503:24)
    at DevServer.runImpl (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/base-server.ts:1103:23)
    at DevServer.run (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/dev/next-dev-server.ts:1245:14)
    at DevServer.handleRequestImpl (/Users/dm/Documents/prediction-buddy-v2/node_modules/next/src/server/base-server.ts:986:14)
null

And the FORCE SEND option still gives:

mailing 500 /api/sendMail 8023ms
- error unhandledRejection: Error: connect ECONNREFUSED 127.0.0.1:587
    at __node_internal_captureLargerStackTrace (node:internal/errors:465:5)
    at __node_internal_exceptionWithHostPort (node:internal/errors:643:12)
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1187:16) {
  errno: -61,
  code: 'ESOCKET',
  syscall: 'connect',
  address: '127.0.0.1',
  port: 587,
  command: 'CONN'
}
psugihara commented 1 year ago

Yes, the preview server calls dotenv.register() which just looks at .env. It works when you send from your app because you’re reading in .env.local into your process.env somehow. To confirm, try moving .env.local contents to .env

danielthedifficult commented 1 year ago

Great, progress.

  1. I updated my package.json to run mailing using export $(grep -v '^#' .env.local | xargs) && npx mailing and it works great.
  2. FYI Next.js uses .env.local as default for dev env vars

Maybe adding a param for a custom .env path in the mailing config would help? You can inject that into the dotenv command at startup as such: if (customDotEnv) dotenv.config({ path: '.env.local'});

or maybe even use it to add on top of the default .env if someone want to make a .env.mailing or something...

dotenv.config(); // Load normal .env vars
if (customDotEnv) dotenv.config({ path: customDotEnvPath, override: true }); // Override any shared keys with those from the new .env
psugihara commented 1 year ago

Good find. I didn't know next was using .env.local. I like the idea of just adding hardcoded .env.local env vars on top without adding an extra config.

dotenv.config(); // Load normal .env vars
dotenv.config({ path: ".env.local", override: true });

Seem good?

danielthedifficult commented 1 year ago

GEFN! And while we're in this conversation, thanks for both your excellent software and your helpfulness.