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

forceSend fails, tries to connect to 587 #474

Open
danielthedifficult opened this issue Jul 17, 2023 · 9 comments
Open

forceSend fails, tries to connect to 587 #474

danielthedifficult opened this issue Jul 17, 2023 · 9 comments

Comments

@danielthedifficult
Copy link

danielthedifficult commented Jul 17, 2023

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Author

danielthedifficult commented Jul 18, 2023

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
Copy link
Collaborator

psugihara commented Jul 19, 2023 via email

@danielthedifficult
Copy link
Author

danielthedifficult commented Jul 19, 2023

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
Copy link
Collaborator

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
Copy link
Author

danielthedifficult commented Jul 20, 2023 via email

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