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

smtp_client: set idleTimeout to 1s < pool_timeout #1842

Merged
merged 2 commits into from Mar 3, 2017

Conversation

Projects
None yet
3 participants
@msimerson
Member

msimerson commented Mar 2, 2017

it prevents zillions of "SMTP connection timed out" errors in the logs

fixes msimerson/Mail-Toaster-6#205

  • s/this/client/ for readability
  • reduce indent
  • only reset arguments with defaults when undefined (vs setting to themselves from a ternary when defined)
smtp_client: set idleTimeout to 1s < pool_timeout
it prevents zillions of "SMTP connection timed out" errors in the logs

fixes msimerson/Mail-Toaster-6#205
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Mar 2, 2017

Codecov Report

Merging #1842 into master will increase coverage by 0.06%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1842      +/-   ##
==========================================
+ Coverage   46.23%   46.29%   +0.06%     
==========================================
  Files          22       22              
  Lines        5863     5864       +1     
  Branches     1474     1474              
==========================================
+ Hits         2711     2715       +4     
+ Misses       3152     3149       -3
Impacted Files Coverage Δ
smtp_client.js 6.82% <0%> (-0.03%)
configfile.js 70% <0%> (+1.17%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa1bb44...c41422c. Read the comment docs.

codecov-io commented Mar 2, 2017

Codecov Report

Merging #1842 into master will increase coverage by 0.06%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1842      +/-   ##
==========================================
+ Coverage   46.23%   46.29%   +0.06%     
==========================================
  Files          22       22              
  Lines        5863     5864       +1     
  Branches     1474     1474              
==========================================
+ Hits         2711     2715       +4     
+ Misses       3152     3149       -3
Impacted Files Coverage Δ
smtp_client.js 6.82% <0%> (-0.03%)
configfile.js 70% <0%> (+1.17%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa1bb44...c41422c. Read the comment docs.

@msimerson msimerson referenced this pull request Mar 2, 2017

Closed

Strange log from Haraka #205

@smfreegard

This comment has been minimized.

Show comment
Hide comment
@smfreegard

smfreegard Mar 2, 2017

Collaborator

Have you actually tested this in production?

You made a similar change to auth_proxy and it's still broken: 081a9ab#diff-0189a1b7e72e585e58f7a3bbd9a797ffL98 (the whole section on SMTP continuations is broken)

Collaborator

smfreegard commented on c41422c Mar 2, 2017

Have you actually tested this in production?

You made a similar change to auth_proxy and it's still broken: 081a9ab#diff-0189a1b7e72e585e58f7a3bbd9a797ffL98 (the whole section on SMTP continuations is broken)

This comment has been minimized.

Show comment
Hide comment
@msimerson

msimerson Mar 2, 2017

Member

Yes, I'm running with this change and some additional debugging right now, trying to determine why client.socket.remoteAddress is sometimes undefined.

Member

msimerson replied Mar 2, 2017

Yes, I'm running with this change and some additional debugging right now, trying to determine why client.socket.remoteAddress is sometimes undefined.

@msimerson msimerson merged commit 51589b3 into haraka:master Mar 3, 2017

2 of 3 checks passed

codecov/patch 0% of diff hit (target 46.23%)
Details
codecov/project 46.29% (+0.06%) compared to fa1bb44
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@msimerson msimerson deleted the msimerson:smtp-client-timeout-errors branch Mar 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment