Our smtp server responded slowly sometimes and reached the timeout, but actually
it had sent the email successfully. The mta then retried and sent duplicated
emails. So have an option to let user to set an appropriate timeout.
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers
Tested in our deployed and it worked
Diff Detail
- Repository
- rP Phabricator
- Branch
- smtp-timeout
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1 Build 1: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Hmm... let's just increase this to a larger value in all cases. I want to avoid adding configuration unless we really need to (we have a huge number of options already, and adding more tends to make it harder to configure Phabricator), so let's just try using a hard-coded (but larger) value first. I'm comfortable with any number up to, say, 300 seconds. Would that be long enough?
That sounds good to me. I actually changed the default values to 60 seconds in hard-code before I composed this code change. 300 seconds is long enough for my situation. But I'm not sure if setting a larger value has side-effect under other situations.
I personally prefer having a configuration option. The user doesn't have to set it if not necessary, so it doesn't increase user's burden, but it makes the potential problem (if it is) more obvious.
Both are ok to me anyway, that's up to you.