Skip to content

fixing redial issue, in case of error we need to dial again #123

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.

Sign up for GitHub

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

Open
arundudeemmt wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fixing redial issue, in case of error we need to dial again #123

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions smtp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,12 @@ type smtpSender struct {

func (c *smtpSender) Send(from string, to []string, msg io.WriterTo) error {
if err := c.Mail(from); err != nil {
if err == io.EOF {
Copy link
Copy Markdown

@ivy ivy Aug 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an important distinction in error handling here: Gomail shouldn't make any attempts to redial unless it was explicitly an EOF. Any failure should instead be handled by the caller.

@arundudeemmt You mentioned in #104 that QOR relies on Gomail. Could you link me to where in the codebase it's used? I'd recommend extending that project to account for errors as it isn't always the best choice to simply redial without any additional work.

Copy link
Copy Markdown
Author

@arundudeemmt arundudeemmt Aug 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some time there are error which are not EOF too and requires a redial like write: broken pipe .For broken pipe i was not able to find any standard error codes from smtp .Please suggest a way to fix the same .
IMO broken pipe comes when there is no event in last x time frame and pipe is broken .So three ways to fix i see it as 1) redial 2)build a keep alive mechanism and keep pinging server after x interval where x is less than time out interval 3)close connection after every email and then redial

Copy link
Copy Markdown
Author

@arundudeemmt arundudeemmt Aug 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivy @pedromorgan did you got a chance to look at this ?

// This is probably due to a timeout, so reconnect and try again.
sc, derr := c.d.Dial()
if derr == nil {
if s, ok := sc.(*smtpSender); ok {
*c = *s
return c.Send(from, to, msg)
}
// This is probably due to a timeout, so reconnect and try again.
sc, derr := c.d.Dial()
if derr == nil {
if s, ok := sc.(*smtpSender); ok {
*c = *s
return c.Send(from, to, msg)
}
}
return err
Expand Down