Fix vpn client

internal/vpn/client.go

@@ -18,6 +18,9 @@ import (
 
 	"github.com/skycoin/skywire/pkg/app"
 	"github.com/skycoin/skywire/pkg/app/appnet"
+	"github.com/skycoin/skywire/pkg/app/appserver"
+	"github.com/skycoin/skywire/pkg/routefinder/rfclient"
+	"github.com/skycoin/skywire/pkg/router"
 	"github.com/skycoin/skywire/pkg/routing"
 	"github.com/skycoin/skywire/pkg/skyenv"
 	skynetutil "github.com/skycoin/skywire/pkg/util/netutil"
@@ -34,7 +37,6 @@ type Client struct {
 	log            *logrus.Logger
 	cfg            ClientConfig
 	appCl          *app.Client
-	r              *netutil.Retrier
 	directIPSMu    sync.Mutex
 	directIPs      []net.IP
 	defaultGateway net.IP
@@ -109,13 +111,7 @@ func NewClient(cfg ClientConfig, appCl *app.Client) (*Client, error) {
 		directIPs = append(directIPs, utIP)
 	}
 
-	const (
-		serverDialInitBO = 1 * time.Second
-		serverDialMaxBO  = 10 * time.Second
-	)
-
 	log := logrus.New()
-	r := netutil.NewRetrier(log, serverDialInitBO, serverDialMaxBO, 0, 1)
 
 	defaultGateway, err := DefaultNetworkGateway()
 	if err != nil {
@@ -128,7 +124,6 @@ func NewClient(cfg ClientConfig, appCl *app.Client) (*Client, error) {
 		log:            log,
 		cfg:            cfg,
 		appCl:          appCl,
-		r:              r,
 		directIPs:      filterOutEqualIPs(directIPs),
 		defaultGateway: defaultGateway,
 		closeC:         make(chan struct{}),
@@ -188,16 +183,35 @@ func (c *Client) Serve() error {
 
 	c.setAppStatus(ClientStatusConnecting)
 
-	r := netutil.NewDefaultRetrier(c.log)
+	errNoTransportFound := appserver.RPCErr{
+		Err: router.ErrNoTransportFound.Error(),
+	}
+
+	errTransportNotFound := appserver.RPCErr{
+		Err: rfclient.ErrTransportNotFound.Error(),
+	}
+
+	r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor).
+		WithErrWhitelist(errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs,
+			errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound)
+
 	err := r.Do(context.Background(), func() error {
 		if c.isClosed() {
 			return nil
 		}
 
 		if err := c.dialServeConn(); err != nil {
-			c.setAppStatus(ClientStatusReconnecting)
-			fmt.Println("Connection broke, reconnecting...")
-			return fmt.Errorf("dialServeConn: %w", err)
+			switch err {
+			case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs,
+				errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound:
+				c.setAppError(err)
+				return err
+			default:
+				c.setAppStatus(ClientStatusReconnecting)
+				c.setAppError(errTimeout)
+				fmt.Println("\nConnection broke, reconnecting...")
+				return fmt.Errorf("dialServeConn: %w", err)
+			}
 		}
 
 		return nil
@@ -326,7 +340,8 @@ func (c *Client) setupTUN(tunIP, tunGateway net.IP) error {
 func (c *Client) serveConn(conn net.Conn) error {
 	tunIP, tunGateway, err := c.shakeHands(conn)
 	if err != nil {
-		return fmt.Errorf("error during client/server handshake: %w", err)
+		fmt.Printf("error during client/server handshake: %s", err)
+		return err
 	}
 
 	fmt.Printf("Performed handshake with %s\n", conn.RemoteAddr())
@@ -429,7 +444,8 @@ func (c *Client) serveConn(conn net.Conn) error {
 func (c *Client) dialServeConn() error {
 	conn, err := c.dialServer(c.appCl, c.cfg.ServerPK)
 	if err != nil {
-		return fmt.Errorf("error connecting to VPN server: %w", err)
+		fmt.Printf("error connecting to VPN server: %s", err)
+		return err
 	}
 
 	fmt.Printf("Dialed %s\n", conn.RemoteAddr())
@@ -445,7 +461,8 @@ func (c *Client) dialServeConn() error {
 	}
 
 	if err := c.serveConn(conn); err != nil {
-		return fmt.Errorf("error serving app conn: %w", err)
+		fmt.Printf("error serving app conn: %s", err)
+		return err
 	}
 
 	return nil
@@ -663,7 +680,7 @@ func (c *Client) shakeHands(conn net.Conn) (TUNIP, TUNGateway net.IP, err error)
 	fmt.Printf("Got server hello: %v", sHello)
 
 	if sHello.Status != HandshakeStatusOK {
-		return nil, nil, fmt.Errorf("got status %d (%s) from the server", sHello.Status, sHello.Status)
+		return nil, nil, sHello.Status.getError()
 	}
 
 	return sHello.TUNIP, sHello.TUNGateway, nil
@@ -684,22 +701,13 @@ func (c *Client) dialServer(appCl *app.Client, pk cipher.PubKey) (net.Conn, erro
 	)
 
 	var conn net.Conn
-	err := c.r.Do(context.Background(), func() error {
-		var err error
-		conn, err = appCl.Dial(appnet.Addr{
-			Net:    netType,
-			PubKey: pk,
-			Port:   vpnPort,
-		})
-
-		if c.isClosed() {
-			// in this case client got closed, we return no error,
-			// so that retrier could stop gracefully
-			return nil
-		}
-
-		return err
+	var err error
+	conn, err = appCl.Dial(appnet.Addr{
+		Net:    netType,
+		PubKey: pk,
+		Port:   vpnPort,
 	})
+
 	if err != nil {
 		return nil, err
 	}
@@ -719,6 +727,12 @@ func (c *Client) setAppStatus(status ClientStatus) {
 	}
 }
 
+func (c *Client) setAppError(appErr error) {
+	if err := c.appCl.SetError(appErr.Error()); err != nil {
+		fmt.Printf("Failed to set error %v: %v\n", appErr, err)
+	}
+}
+
 func (c *Client) isClosed() bool {
 	select {
 	case <-c.closeC:

internal/vpn/errors.go

@@ -3,5 +3,10 @@ package vpn
 import "errors"
 
 var (
-	errCouldFindDefaultNetworkGateway = errors.New("could not find default network gateway")
+	errCouldFindDefaultNetworkGateway = errors.New("Could not find default network gateway")
+	errHandshakeStatusForbidden       = errors.New("Password didn't match")
+	errHandshakeStatusInternalError   = errors.New("Internal server error")
+	errHandshakeNoFreeIPs             = errors.New("No free IPs left to serve")
+	errHandshakeStatusBadRequest      = errors.New("Request was malformed")
+	errTimeout                        = errors.New("Internal error: Timeout")
 )

internal/vpn/handshake_status.go

@@ -1,5 +1,7 @@
 package vpn
 
+import "errors"
+
 // HandshakeStatus is a status of Client/Server handshake.
 type HandshakeStatus int
 
@@ -32,3 +34,20 @@ func (hs HandshakeStatus) String() string {
 		return "Unknown code"
 	}
 }
+
+func (hs HandshakeStatus) getError() error {
+	switch hs {
+	case HandshakeStatusOK:
+		return nil
+	case HandshakeStatusBadRequest:
+		return errHandshakeStatusBadRequest
+	case HandshakeNoFreeIPs:
+		return errHandshakeNoFreeIPs
+	case HandshakeStatusInternalError:
+		return errHandshakeStatusInternalError
+	case HandshakeStatusForbidden:
+		return errHandshakeStatusForbidden
+	default:
+		return errors.New("Unknown error code")
+	}
+}

pkg/app/appserver/proc.go

@@ -53,6 +53,8 @@ type Proc struct {
 
 	statusMx sync.RWMutex
 	status   string
+	errMx    sync.RWMutex
+	err      string
 }
 
 // NewProc constructs `Proc`.
@@ -170,7 +172,8 @@ func (p *Proc) Start() error {
 			// here will definitely be an error notifying that the process
 			// is already stopped. We do this to remove proc from the manager,
 			// therefore giving the correct app status to hypervisor.
-			_ = p.m.Stop(p.appName) //nolint:errcheck
+			_ = p.m.SetError(p.appName, p.err) //nolint:errcheck
+			_ = p.m.Stop(p.appName)            //nolint:errcheck
 		}()
 
 		select {
@@ -285,6 +288,22 @@ func (p *Proc) DetailedStatus() string {
 	return p.status
 }
 
+// SetError sets proc's detailed status error.
+func (p *Proc) SetError(appErr string) {
+	p.errMx.Lock()
+	defer p.errMx.Unlock()
+
+	p.err = appErr
+}
+
+// Error gets proc's error.
+func (p *Proc) Error() string {
+	p.errMx.RLock()
+	defer p.errMx.RUnlock()
+
+	return p.err
+}
+
 // ConnectionSummary sums up the connection stats.
 type ConnectionSummary struct {
 	IsAlive           bool          `json:"is_alive"`

pkg/app/appserver/proc_manager.go

@@ -38,6 +38,8 @@ type ProcManager interface {
 	io.Closer
 	Start(conf appcommon.ProcConfig) (appcommon.ProcID, error)
 	ProcByName(appName string) (*Proc, bool)
+	SetError(appName, status string) error
+	ErrorByName(appName string) (string, bool)
 	Stop(appName string) error
 	Wait(appName string) error
 	Range(next func(appName string, proc *Proc) bool)
@@ -61,6 +63,7 @@ type procManager struct {
 	procs      map[string]*Proc
 	procsByKey map[appcommon.ProcKey]*Proc
 
+	errors map[string]string
 	// event broadcaster: broadcasts events to apps
 	eb *appevent.Broadcaster
 
@@ -93,6 +96,7 @@ func NewProcManager(mLog *logging.MasterLogger, discF *appdisc.Factory, eb *appe
 		discF:      discF,
 		procs:      make(map[string]*Proc),
 		procsByKey: make(map[appcommon.ProcKey]*Proc),
+		errors:     make(map[string]string),
 		eb:         eb,
 		done:       make(chan struct{}),
 	}
@@ -209,7 +213,7 @@ func (m *procManager) Start(conf appcommon.ProcConfig) (appcommon.ProcID, error)
 
 		return 0, err
 	}
-
+	delete(m.errors, conf.AppName)
 	return appcommon.ProcID(proc.cmd.Process.Pid), nil
 }
 
@@ -221,6 +225,14 @@ func (m *procManager) ProcByName(appName string) (*Proc, bool) {
 	return proc, ok
 }
 
+func (m *procManager) ErrorByName(appName string) (string, bool) {
+	m.mx.RLock()
+	defer m.mx.RUnlock()
+
+	err, ok := m.errors[appName]
+	return err, ok
+}
+
 // Stop stops the application.
 func (m *procManager) Stop(name string) error {
 	p, err := m.pop(name)
@@ -245,14 +257,14 @@ func (m *procManager) Wait(name string) error {
 			err = fmt.Errorf("failed to run app executable %s: %w", name, err)
 		}
 
-		if _, err := m.pop(name); err != nil {
+		if _, err := m.pop(name); err != nil { //nolint:errcheck
 			m.log.Debugf("Remove app <%v>: %v", name, err)
 		}
 
 		return err
 	}
 
-	_, err = m.pop(name)
+	_, err = m.pop(name) //nolint:errcheck
 
 	return err
 }
@@ -310,6 +322,16 @@ func (m *procManager) DetailedStatus(appName string) (string, error) {
 	return p.DetailedStatus(), nil
 }
 
+// SetError sets error `appErr` for app `appName`.
+func (m *procManager) SetError(appName, appErr string) error {
+	m.mx.Lock()
+	defer m.mx.Unlock()
+
+	m.errors[appName] = appErr
+
+	return nil
+}
+
 // ConnectionsSummary gets connections info for the app `appName`.
 func (m *procManager) ConnectionsSummary(appName string) ([]ConnectionSummary, error) {
 	p, err := m.get(appName)

pkg/app/appserver/proc_manager_test.go

@@ -16,14 +16,14 @@ func TestProcManager_ProcByName(t *testing.T) {
 
 	appName := "app"
 
-	_, ok = m.ProcByName(appName)
+	_, ok = m.ProcByName(appName) //nolint:errcheck
 	require.False(t, ok)
 
 	m.mx.Lock()
 	m.procs[appName] = nil
 	m.mx.Unlock()
 
-	_, ok = m.ProcByName(appName)
+	_, ok = m.ProcByName(appName) //nolint:errcheck
 	require.True(t, ok)
 }