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

Retry Connection reset? #43

Open
StevenACoffman opened this issue Sep 30, 2020 · 3 comments
Open

Retry Connection reset? #43

StevenACoffman opened this issue Sep 30, 2020 · 3 comments

Comments

@StevenACoffman
Copy link

When I am making GET requests using Pester, I sporadically get connection reset by peer errors:

read tcp 10.24.8.45:37286->13.49.93.125:443 read: connection reset by peer

I can detect these errors using:

if opErr, ok := err.(*net.OpError); ok {
    if syscallErr, ok := opErr.Err.(*os.SyscallError); ok {
        if syscallErr.Err == syscall.ECONNRESET {
            fmt.Println("Found a ECONNRESET")
        }
    }
}

I would like Pester to retry these requests. What do you think?

@sethgrid
Copy link
Owner

For connection reset by peer, I think having pester retry those makes a lot of sense. PRs very welcome!

@StevenACoffman
Copy link
Author

I'm on support this week, so I may get unexpectedly sucked into something and be unable to come back to this. Sorry for the spam, but if it's ok, I'm putting more notes in here.

package main

import (
	"fmt"
	"log"
	"net"
	"os"
	"syscall"
	"time"
)

// FIN vs RST Experiment
// ---
// Demonstrates how to trigger a connection reset by peer on close
// using SetLinger.
// ---
// $ go run experiment.go
// 2019/01/02 08:29:36 connect err: EOF
// exit status 1
// $ go run experiment.go rst
// 2019/01/02 08:29:40 connect err: read tcp 127.0.0.1:50361->127.0.0.1:4444: read: connection reset by peer
// exit status 1

func main() {
	rst := len(os.Args) >= 2 && os.Args[1] == "rst"

	// Start a listener that accepts one connection and then immediately closes it.
	go func() {
		err := listen(rst)
		if err != nil {
			log.Fatalf("listen err: %s", err)
		}
	}()

	// Wait for listener to start.
	time.Sleep(time.Second)

	// Connect and report error on close.
	err := connect()
	if err != nil {
		log.Fatalf("connect err: %s", err)
	}
}

func listen(rst bool) error {
	ln, err := net.Listen("tcp", "localhost:4444")
	if err != nil {
		return err
	}

	// Accept one connection.
	conn, err := ln.Accept()
	if err != nil {
		return err
	}

	// Force an RST if flag is set.
	// Close with normal FIN if flag is not set.
	if rst {
		tcpConn := conn.(*net.TCPConn)
		tcpConn.SetLinger(0)
	}
	return conn.Close()
}

func connect() error {
	conn, err := net.Dial("tcp", "localhost:4444")
	if err != nil {
		return err
	}
	// Block until close.
	buf := make([]byte, 1)
	_, err = conn.Read(buf)
	if err != nil {
		if opErr, ok := err.(*net.OpError); ok {
			if syscallErr, ok := opErr.Err.(*os.SyscallError); ok {
				if syscallErr.Err == syscall.ECONNRESET {
					fmt.Println("Found an ECONNRESET error")
				}
			}
		}
	}
	return err
}

This is how I'm simulating connection resets to be be able to test that I'm catching them properly.

@StevenACoffman
Copy link
Author

I wonder if it might be easier to simply retry any error that isn't one of the few non-transient errors:

	if err != nil {
		if v, ok := err.(*url.Error); ok {
			// Don't retry if the error was due to too many redirects.
			if redirectsErrorRe.MatchString(v.Error()) {
				return false, nil
			}

			// Don't retry if the error was due to an invalid protocol scheme.
			if schemeErrorRe.MatchString(v.Error()) {
				return false, nil
			}

			// Don't retry if the error was due to TLS cert verification failure.
			if _, ok := v.Err.(x509.UnknownAuthorityError); ok {
				return false, nil
			}
		}

		// The error is likely recoverable so retry.
		return true, nil
	}

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

No branches or pull requests

2 participants