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

Consider ECONNRESET a non-temporary error. #287

Closed
wants to merge 1 commit into from

Conversation

luketurner
Copy link
Contributor

This PR ensures that the syslog adapter treats ECONNRESET errors as "non-temporary", meaning that we immediately recreate the connection instead of first attempting to re-send the failed packets on the existing connection.

It should be noted, there are cases ("under certain rapid connection setup/teardown conditions") where ECONNRESET can be a temporary error. See the Golang issue and associated commit.

However, it seems to me that of the real-world cases where an ECONNRESET is encountered, the great majority of the problems would be non-temporary. For example, Papertrail delivers an ECONNRESET after 15 minutes of inactivity, which indicates that they have permanently closed the connection and we need to open a new one.

@mattatcha
Copy link
Member

I think we should try using write/read deadlines before accepting this. Ive included a link to docs about SetDeadline.

        // SetDeadline sets the read and write deadlines associated
        // with the connection. It is equivalent to calling both
        // SetReadDeadline and SetWriteDeadline.
        //
        // A deadline is an absolute time after which I/O operations
        // fail with a timeout (see type Error) instead of
        // blocking. The deadline applies to all future and pending
        // I/O, not just the immediately following call to Read or
        // Write. After a deadline has been exceeded, the connection
        // can be refreshed by setting a deadline in the future.
        //
        // An idle timeout can be implemented by repeatedly extending
        // the deadline after successful Read or Write calls.
        //
        // A zero value for t means I/O operations will not time out.
        SetDeadline(t time.Time) error

https://golang.org/pkg/net/#Conn

Copy link
Member

@mattatcha mattatcha left a comment

Choose a reason for hiding this comment

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

See comment

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

Successfully merging this pull request may close these issues.

3 participants