Skip to content

/ rails

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

Jump to bottom

ActionMailer does not timeout network connections by default #41244

Open
mperham opened this issue Jan 26, 2021 · 3 comments · May be fixed by #41248
Open

ActionMailer does not timeout network connections by default #41244

mperham opened this issue Jan 26, 2021 · 3 comments · May be fixed by #41248
Labels
actionmailer good first issue

Comments

@mperham
Copy link
Contributor

@mperham mperham commented Jan 26, 2021

I have a Sidekiq customer complaining of a lot of "stuck" threads. He sends a lot of email so I suspected misbehaving SMTP servers.

The mail gem has support for timeouts here but they default to nil:

https://github.com/mikel/mail/blob/7b3e100f42f2d7738c3af7bf1909777568270b67/lib/mail/network/delivery_methods/smtp.rb#L91

Rails does not appear to set them by default:

rails/actionmailer/lib/action_mailer/delivery_methods.rb

Lines 20 to 27 in a090847

add_delivery_method :smtp, Mail::SMTP,
address: "localhost",
port: 25,
domain: "localhost.localdomain",
user_name: nil,
password: nil,
authentication: nil,
enable_starttls_auto: true

nor does it appear to document how to set timeouts in the SMTP configuration section:

https://guides.rubyonrails.org/action_mailer_basics.html#action-mailer-configuration

Would a read/open timeout of 5 seconds be reasonable to use as a default going forward? Could we update the ActionMailer guide for 6.1 so people know how to configure the timeouts?
@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Jan 26, 2021

Would a read/open timeout of 5 seconds be reasonable to use as a default going forward?

Yes

Could we update the ActionMailer guide for 6.1 so people know how to configure the timeouts?

Yes

I think we should do both.

I marked the issue with the "Good first issue", but if you want to do or get your customer involved with contributing to Rails, please go ahead.
@andrehjr
Copy link
Contributor

@andrehjr andrehjr commented Jan 26, 2021

Unless @mperham wants to do it. I can make that PR 🙌
@mperham
Copy link
Contributor Author

@mperham mperham commented Jan 26, 2021

@andrehjr andrehjr linked a pull request that will close this issue Jan 26, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
actionmailer good first issue
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Document open_timeout and read_timeout for SMTP Settings on ActionMailer
3 participants
@mperham @andrehjr @rafaelfranca