From ab0a25a92435e1dd048dffa37b5805cdc72769e6 Mon Sep 17 00:00:00 2001 From: ersonp Date: Thu, 30 Sep 2021 12:15:08 +0530 Subject: [PATCH 01/19] Update retrier We update the retirer to not retry till success anymore. We only retry three times. --- internal/vpn/client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index b4711da66c..025e1702f4 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -188,7 +188,8 @@ func (c *Client) Serve() error { c.setAppStatus(ClientStatusConnecting) - r := netutil.NewDefaultRetrier(c.log) + r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor) + err := r.Do(context.Background(), func() error { if c.isClosed() { return nil From b192e36fa671b474386a5bc7fc625ac1b22ef317 Mon Sep 17 00:00:00 2001 From: ersonp Date: Thu, 30 Sep 2021 16:45:42 +0530 Subject: [PATCH 02/19] Add whitelisted errors We whitelist errors sent from server hello as it's pointless to retry. --- internal/vpn/client.go | 23 ++++++++++++++++++++--- internal/vpn/errors.go | 4 ++++ internal/vpn/handshake_status.go | 19 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 025e1702f4..87c1136cbc 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -188,7 +188,9 @@ func (c *Client) Serve() error { c.setAppStatus(ClientStatusConnecting) - r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor) + r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor). + WithErrWhitelist(errHandshakeStatusForbidden).WithErrWhitelist(errHandshakeStatusInternalError). + WithErrWhitelist(errHandshakeNoFreeIPs).WithErrWhitelist(errHandshakeStatusBadRequest) err := r.Do(context.Background(), func() error { if c.isClosed() { @@ -197,7 +199,12 @@ func (c *Client) Serve() error { if err := c.dialServeConn(); err != nil { c.setAppStatus(ClientStatusReconnecting) - fmt.Println("Connection broke, reconnecting...") + fmt.Println("\nConnection broke, reconnecting...") + switch err { + case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, + errHandshakeStatusBadRequest: + return err + } return fmt.Errorf("dialServeConn: %w", err) } @@ -327,6 +334,11 @@ 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 { + switch err { + case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, + errHandshakeStatusBadRequest: + return err + } return fmt.Errorf("error during client/server handshake: %w", err) } @@ -446,6 +458,11 @@ func (c *Client) dialServeConn() error { } if err := c.serveConn(conn); err != nil { + switch err { + case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, + errHandshakeStatusBadRequest: + return err + } return fmt.Errorf("error serving app conn: %w", err) } @@ -664,7 +681,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, HandshakeStatusForbidden.getError() } return sHello.TUNIP, sHello.TUNGateway, nil diff --git a/internal/vpn/errors.go b/internal/vpn/errors.go index 06b6264f83..238f980f1c 100644 --- a/internal/vpn/errors.go +++ b/internal/vpn/errors.go @@ -4,4 +4,8 @@ import "errors" var ( 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") ) diff --git a/internal/vpn/handshake_status.go b/internal/vpn/handshake_status.go index 8fad8983a8..5463e2bde0 100644 --- a/internal/vpn/handshake_status.go +++ b/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 code") + } +} From 082c4fa76ca1b98ce9380a60cb33264f2995eead Mon Sep 17 00:00:00 2001 From: ersonp Date: Thu, 30 Sep 2021 20:34:43 +0530 Subject: [PATCH 03/19] Remove conSummarry check The commit contains changes to the logic of AppStates(). We are removing the connection summary check as whenever a vpn-client gets an error it closes all the connections to retry again. Because of this even when the vpn-client is still running AppStates() gives the app status for vpn-client as stopped. Insted depending on ProcByName() is sufficient. --- pkg/app/launcher/launcher.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/app/launcher/launcher.go b/pkg/app/launcher/launcher.go index d67f6fee0d..c55b458e5d 100644 --- a/pkg/app/launcher/launcher.go +++ b/pkg/app/launcher/launcher.go @@ -189,10 +189,7 @@ func (l *Launcher) AppStates() []*AppState { state := &AppState{AppConfig: app, Status: AppStatusStopped} if proc, ok := l.procM.ProcByName(app.Name); ok { state.DetailedStatus = proc.DetailedStatus() - connSummary := proc.ConnectionsSummary() - if connSummary != nil { - state.Status = AppStatusRunning - } + state.Status = AppStatusRunning } states = append(states, state) } From a0d8f0d2031dc4665f167266a76acb8b17ebe04a Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 15:47:02 +0530 Subject: [PATCH 04/19] Add app error The commit contains additions to the proc and rpc code. The addition is of the app status AppStatusErrored that is set if there is an entry in statusErr which is set if the app encounters an error and stops. A . The commit only makes required changes in vpn-client to set the error. --- internal/vpn/client.go | 15 +++++++++--- internal/vpn/errors.go | 5 ++-- pkg/app/appserver/proc.go | 25 +++++++++++++++++--- pkg/app/appserver/proc_manager.go | 29 +++++++++++++++++++++--- pkg/app/appserver/rpc_ingress_client.go | 6 +++++ pkg/app/appserver/rpc_ingress_gateway.go | 9 ++++++++ pkg/app/client.go | 5 ++++ pkg/app/launcher/app_state.go | 3 +++ pkg/app/launcher/launcher.go | 7 ++++++ pkg/visor/api.go | 14 ++++++++++++ pkg/visor/rpc.go | 13 ++++++++--- pkg/visor/rpc_client.go | 24 +++++++++++++++++++- 12 files changed, 140 insertions(+), 15 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 87c1136cbc..5316c17ded 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -198,14 +198,17 @@ func (c *Client) Serve() error { } if err := c.dialServeConn(); err != nil { - c.setAppStatus(ClientStatusReconnecting) - fmt.Println("\nConnection broke, reconnecting...") switch err { case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, errHandshakeStatusBadRequest: + c.setAppStatusError(ClientStatus(err.Error())) return err + default: + c.setAppStatus(ClientStatusReconnecting) + c.setAppStatusError(ClientStatus(errTimeout.Error())) + fmt.Println("\nConnection broke, reconnecting...") + return fmt.Errorf("dialServeConn: %w", err) } - return fmt.Errorf("dialServeConn: %w", err) } return nil @@ -737,6 +740,12 @@ func (c *Client) setAppStatus(status ClientStatus) { } } +func (c *Client) setAppStatusError(statusErr ClientStatus) { + if err := c.appCl.SetDetailedStatusError(string(statusErr)); err != nil { + fmt.Printf("Failed to set status error %v: %v\n", statusErr, err) + } +} + func (c *Client) isClosed() bool { select { case <-c.closeC: diff --git a/internal/vpn/errors.go b/internal/vpn/errors.go index 238f980f1c..30ec471af5 100644 --- a/internal/vpn/errors.go +++ b/internal/vpn/errors.go @@ -3,9 +3,10 @@ package vpn import "errors" var ( - errCouldFindDefaultNetworkGateway = errors.New("could not find default network gateway") - errHandshakeStatusForbidden = errors.New("password didn't match") + 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") ) diff --git a/pkg/app/appserver/proc.go b/pkg/app/appserver/proc.go index ca14e50476..3a4037d1b8 100644 --- a/pkg/app/appserver/proc.go +++ b/pkg/app/appserver/proc.go @@ -51,8 +51,10 @@ type Proc struct { startTimeMx sync.RWMutex startTime time.Time - statusMx sync.RWMutex - status string + statusMx sync.RWMutex + status string + errMx sync.RWMutex + statusErr 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.SetDetailedStatusError(p.appName, p.statusErr) + _ = p.m.Stop(p.appName) }() select { @@ -285,6 +288,22 @@ func (p *Proc) DetailedStatus() string { return p.status } +// SetDetailedStatusError sets proc's detailed status error. +func (p *Proc) SetDetailedStatusError(statusErr string) { + p.errMx.Lock() + defer p.errMx.Unlock() + + p.statusErr = statusErr +} + +// Error gets proc's error. +func (p *Proc) Error() string { + p.errMx.RLock() + defer p.errMx.RUnlock() + + return p.statusErr +} + // ConnectionSummary sums up the connection stats. type ConnectionSummary struct { IsAlive bool `json:"is_alive"` diff --git a/pkg/app/appserver/proc_manager.go b/pkg/app/appserver/proc_manager.go index 2992fcd60e..7ee144970e 100644 --- a/pkg/app/appserver/proc_manager.go +++ b/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) + SetDetailedStatusError(appName, status string) error + DetailedStatusErrorByName(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,8 +225,17 @@ func (m *procManager) ProcByName(appName string) (*Proc, bool) { return proc, ok } +func (m *procManager) DetailedStatusErrorByName(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 { + m.log.Error("111111111111111111111111111111111111") p, err := m.pop(name) if err != nil { return err @@ -245,14 +258,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 +323,16 @@ func (m *procManager) DetailedStatus(appName string) (string, error) { return p.DetailedStatus(), nil } +// SetDetailedStatusError error `statusErr` for app `appName`. +func (m *procManager) SetDetailedStatusError(appName, statusErr string) error { + m.mx.RLock() + defer m.mx.RUnlock() + + m.errors[appName] = statusErr + + return nil +} + // ConnectionsSummary gets connections info for the app `appName`. func (m *procManager) ConnectionsSummary(appName string) ([]ConnectionSummary, error) { p, err := m.get(appName) diff --git a/pkg/app/appserver/rpc_ingress_client.go b/pkg/app/appserver/rpc_ingress_client.go index 2558da1a82..8480b16075 100644 --- a/pkg/app/appserver/rpc_ingress_client.go +++ b/pkg/app/appserver/rpc_ingress_client.go @@ -15,6 +15,7 @@ import ( // RPCIngressClient describes RPC interface to communicate with the server. type RPCIngressClient interface { SetDetailedStatus(status string) error + SetDetailedStatusError(aErr string) error Dial(remote appnet.Addr) (connID uint16, localPort routing.Port, err error) Listen(local appnet.Addr) (uint16, error) Accept(lisID uint16) (connID uint16, remote appnet.Addr, err error) @@ -46,6 +47,11 @@ func (c *rpcIngressClient) SetDetailedStatus(status string) error { return c.rpc.Call(c.formatMethod("SetDetailedStatus"), &status, nil) } +// SetDetailedStatusError sets detailed status error of an app. +func (c *rpcIngressClient) SetDetailedStatusError(statusErr string) error { + return c.rpc.Call(c.formatMethod("SetDetailedStatusError"), &statusErr, nil) +} + // Dial sends `Dial` command to the server. func (c *rpcIngressClient) Dial(remote appnet.Addr) (connID uint16, localPort routing.Port, err error) { var resp DialResp diff --git a/pkg/app/appserver/rpc_ingress_gateway.go b/pkg/app/appserver/rpc_ingress_gateway.go index b452ebf590..c5ad2c723a 100644 --- a/pkg/app/appserver/rpc_ingress_gateway.go +++ b/pkg/app/appserver/rpc_ingress_gateway.go @@ -85,6 +85,15 @@ func (r *RPCIngressGateway) SetDetailedStatus(status *string, _ *struct{}) (err return nil } +// SetDetailedStatusError sets error of an app. +func (r *RPCIngressGateway) SetDetailedStatusError(aErr *string, _ *struct{}) (err error) { + defer rpcutil.LogCall(r.log, "SetDetailedStatusError", aErr)(nil, &err) + + r.proc.SetDetailedStatusError(*aErr) + + return nil +} + // DialResp contains response parameters for `Dial`. type DialResp struct { ConnID uint16 diff --git a/pkg/app/client.go b/pkg/app/client.go index 8ad159586d..8df262dffb 100644 --- a/pkg/app/client.go +++ b/pkg/app/client.go @@ -68,6 +68,11 @@ func (c *Client) SetDetailedStatus(status string) error { return c.rpcC.SetDetailedStatus(status) } +// SetDetailedStatusError sets detailed app status within the visor. +func (c *Client) SetDetailedStatusError(aErr string) error { + return c.rpcC.SetDetailedStatusError(aErr) +} + // Dial dials the remote visor using `remote`. func (c *Client) Dial(remote appnet.Addr) (net.Conn, error) { connID, localPort, err := c.rpcC.Dial(remote) diff --git a/pkg/app/launcher/app_state.go b/pkg/app/launcher/app_state.go index d48f11e8e9..3ff33130a0 100644 --- a/pkg/app/launcher/app_state.go +++ b/pkg/app/launcher/app_state.go @@ -9,6 +9,9 @@ const ( // AppStatusRunning represents status of a running App. AppStatusRunning + + // AppStatusErrored represents status of an errored App. + AppStatusErrored ) // AppState defines state parameters for a registered App. diff --git a/pkg/app/launcher/launcher.go b/pkg/app/launcher/launcher.go index c55b458e5d..97a97797e2 100644 --- a/pkg/app/launcher/launcher.go +++ b/pkg/app/launcher/launcher.go @@ -173,6 +173,9 @@ func (l *Launcher) AppState(name string) (*AppState, bool) { return nil, false } state := &AppState{AppConfig: ac, Status: AppStatusStopped} + if _, ok := l.procM.DetailedStatusErrorByName(ac.Name); ok { + state.Status = AppStatusErrored + } if _, ok := l.procM.ProcByName(ac.Name); ok { state.Status = AppStatusRunning } @@ -187,6 +190,10 @@ func (l *Launcher) AppStates() []*AppState { var states []*AppState for _, app := range l.apps { state := &AppState{AppConfig: app, Status: AppStatusStopped} + if err, ok := l.procM.DetailedStatusErrorByName(app.Name); ok { + state.DetailedStatus = err + state.Status = AppStatusErrored + } if proc, ok := l.procM.ProcByName(app.Name); ok { state.DetailedStatus = proc.DetailedStatus() state.Status = AppStatusRunning diff --git a/pkg/visor/api.go b/pkg/visor/api.go index e8dbffec95..18c9e00c82 100644 --- a/pkg/visor/api.go +++ b/pkg/visor/api.go @@ -40,6 +40,7 @@ type API interface { StartApp(appName string) error StopApp(appName string) error SetAppDetailedStatus(appName, state string) error + SetAppDetailedStatusError(appName, stateErr string) error RestartApp(appName string) error SetAutoStart(appName string, autostart bool) error SetAppPassword(appName, password string) error @@ -349,6 +350,19 @@ func (v *Visor) SetAppDetailedStatus(appName, status string) error { return nil } +// SetAppDetailedStatusError implements API. +func (v *Visor) SetAppDetailedStatusError(appName, aErr string) error { + proc, ok := v.procM.ProcByName(appName) + if !ok { + return ErrAppProcNotRunning + } + + v.log.Infof("Setting app detailed status error %v for app %v", aErr, appName) + proc.SetDetailedStatusError(aErr) + + return nil +} + // RestartApp implements API. func (v *Visor) RestartApp(appName string) error { if _, ok := v.procM.ProcByName(appName); ok { diff --git a/pkg/visor/rpc.go b/pkg/visor/rpc.go index e0c38b5892..69a219dec6 100644 --- a/pkg/visor/rpc.go +++ b/pkg/visor/rpc.go @@ -170,19 +170,26 @@ func (r *RPC) Overview(_ *struct{}, out *Overview) (err error) { <<< APP MANAGEMENT >>> */ -// SetAppDetailedStatusIn is input for SetAppDetailedStatus. -type SetAppDetailedStatusIn struct { +// SetAppStatusIn is input for SetAppDetailedStatus and SetAppDetailedStatusError. +type SetAppStatusIn struct { AppName string Status string } // SetAppDetailedStatus sets app's detailed status. -func (r *RPC) SetAppDetailedStatus(in *SetAppDetailedStatusIn, _ *struct{}) (err error) { +func (r *RPC) SetAppDetailedStatus(in *SetAppStatusIn, _ *struct{}) (err error) { defer rpcutil.LogCall(r.log, "SetAppDetailedStatus", in)(nil, &err) return r.visor.SetAppDetailedStatus(in.AppName, in.Status) } +// SetAppDetailedStatusError sets app's detailed status error. +func (r *RPC) SetAppDetailedStatusError(in *SetAppStatusIn, _ *struct{}) (err error) { + defer rpcutil.LogCall(r.log, "SetAppDetailedStatusError", in)(nil, &err) + + return r.visor.SetAppDetailedStatusError(in.AppName, in.Status) +} + // Apps returns list of Apps registered on the Visor. func (r *RPC) Apps(_ *struct{}, reply *[]*launcher.AppState) (err error) { defer rpcutil.LogCall(r.log, "Apps", nil)(reply, &err) diff --git a/pkg/visor/rpc_client.go b/pkg/visor/rpc_client.go index 6a8f993e0e..4e4b530ddd 100644 --- a/pkg/visor/rpc_client.go +++ b/pkg/visor/rpc_client.go @@ -140,12 +140,20 @@ func (rc *rpcClient) StopApp(appName string) error { // SetAppDetailedStatus sets app's detailed state. func (rc *rpcClient) SetAppDetailedStatus(appName, status string) error { - return rc.Call("SetAppDetailedStatus", &SetAppDetailedStatusIn{ + return rc.Call("SetAppDetailedStatus", &SetAppStatusIn{ AppName: appName, Status: status, }, &struct{}{}) } +// SetAppDetailedStatusError sets app's detailed status error. +func (rc *rpcClient) SetAppDetailedStatusError(appName, statusErr string) error { + return rc.Call("SetAppDetailedStatusError", &SetAppStatusIn{ + AppName: appName, + Status: statusErr, + }, &struct{}{}) +} + // RestartApp calls `RestartApp`. func (rc *rpcClient) RestartApp(appName string) error { return rc.Call("RestartApp", &appName, &struct{}{}) @@ -678,6 +686,20 @@ func (mc *mockRPCClient) SetAppDetailedStatus(appName, status string) error { }) } +// SetAppDetailedStatusError sets app's detailed state error. +func (mc *mockRPCClient) SetAppDetailedStatusError(appName, aErr string) error { + return mc.do(true, func() error { + for _, a := range mc.o.Apps { + if a.Name == appName { + a.DetailedStatus = aErr + return nil + } + } + + return fmt.Errorf("app of name '%s' does not exist", appName) + }) +} + // RestartApp implements API. func (*mockRPCClient) RestartApp(string) error { return nil From 909da77fab98aaafab80fe960ddb8a3143ffac77 Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 15:49:16 +0530 Subject: [PATCH 05/19] Update MockRPCIngressClient This commit contains updates to MockRPCIngressClient after the changes to RPCIngressClient in previous commit. The code was auto generated via mockery. --- pkg/app/appserver/mock_rpc_ingress_client.go | 22 ++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/pkg/app/appserver/mock_rpc_ingress_client.go b/pkg/app/appserver/mock_rpc_ingress_client.go index 4f989a2db6..3447094700 100644 --- a/pkg/app/appserver/mock_rpc_ingress_client.go +++ b/pkg/app/appserver/mock_rpc_ingress_client.go @@ -1,14 +1,14 @@ -// Code generated by mockery v1.0.0. DO NOT EDIT. +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. package appserver import ( - time "time" - + appnet "github.com/skycoin/skywire/pkg/app/appnet" mock "github.com/stretchr/testify/mock" - appnet "github.com/skycoin/skywire/pkg/app/appnet" routing "github.com/skycoin/skywire/pkg/routing" + + time "time" ) // MockRPCIngressClient is an autogenerated mock type for the RPCIngressClient type @@ -170,6 +170,20 @@ func (_m *MockRPCIngressClient) SetDetailedStatus(status string) error { return r0 } +// SetDetailedStatusError provides a mock function with given fields: aErr +func (_m *MockRPCIngressClient) SetDetailedStatusError(aErr string) error { + ret := _m.Called(aErr) + + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { + r0 = rf(aErr) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // SetReadDeadline provides a mock function with given fields: connID, d func (_m *MockRPCIngressClient) SetReadDeadline(connID uint16, d time.Time) error { ret := _m.Called(connID, d) From 0be96456b701bddb3c4afdd2cd6e1ae5b0930aa1 Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 15:50:42 +0530 Subject: [PATCH 06/19] Add nolint:errcheck This commit contains nolint:errcheck wherever the return value from a func was not checked to suppress warnings. --- pkg/app/appserver/proc.go | 4 ++-- pkg/app/appserver/proc_manager_test.go | 4 ++-- pkg/app/launcher/launcher.go | 4 ++-- pkg/visor/api.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/app/appserver/proc.go b/pkg/app/appserver/proc.go index 3a4037d1b8..0c00f7149d 100644 --- a/pkg/app/appserver/proc.go +++ b/pkg/app/appserver/proc.go @@ -172,8 +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.SetDetailedStatusError(p.appName, p.statusErr) - _ = p.m.Stop(p.appName) + _ = p.m.SetDetailedStatusError(p.appName, p.statusErr) //nolint:errcheck + _ = p.m.Stop(p.appName) //nolint:errcheck }() select { diff --git a/pkg/app/appserver/proc_manager_test.go b/pkg/app/appserver/proc_manager_test.go index 5f4a63ba22..7bd1a0592c 100644 --- a/pkg/app/appserver/proc_manager_test.go +++ b/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) } diff --git a/pkg/app/launcher/launcher.go b/pkg/app/launcher/launcher.go index 97a97797e2..7d353f66bb 100644 --- a/pkg/app/launcher/launcher.go +++ b/pkg/app/launcher/launcher.go @@ -173,10 +173,10 @@ func (l *Launcher) AppState(name string) (*AppState, bool) { return nil, false } state := &AppState{AppConfig: ac, Status: AppStatusStopped} - if _, ok := l.procM.DetailedStatusErrorByName(ac.Name); ok { + if _, ok := l.procM.DetailedStatusErrorByName(ac.Name); ok { //nolint:errcheck state.Status = AppStatusErrored } - if _, ok := l.procM.ProcByName(ac.Name); ok { + if _, ok := l.procM.ProcByName(ac.Name); ok { //nolint:errcheck state.Status = AppStatusRunning } return state, true diff --git a/pkg/visor/api.go b/pkg/visor/api.go index 18c9e00c82..ffaf1a3328 100644 --- a/pkg/visor/api.go +++ b/pkg/visor/api.go @@ -333,7 +333,7 @@ func (v *Visor) StartApp(appName string) error { // StopApp implements API. func (v *Visor) StopApp(appName string) error { - _, err := v.appL.StopApp(appName) + _, err := v.appL.StopApp(appName) //nolint:errcheck return err } @@ -365,7 +365,7 @@ func (v *Visor) SetAppDetailedStatusError(appName, aErr string) error { // RestartApp implements API. func (v *Visor) RestartApp(appName string) error { - if _, ok := v.procM.ProcByName(appName); ok { + if _, ok := v.procM.ProcByName(appName); ok { //nolint:errcheck v.log.Infof("Updated %v password, restarting it", appName) return v.appL.RestartApp(appName) } From c1689930a9bdabcfaec219de728e44d8033219fb Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 15:54:41 +0530 Subject: [PATCH 07/19] Remove test log This commit removes log that was used for testing. --- pkg/app/appserver/proc_manager.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/app/appserver/proc_manager.go b/pkg/app/appserver/proc_manager.go index 7ee144970e..d5aff85640 100644 --- a/pkg/app/appserver/proc_manager.go +++ b/pkg/app/appserver/proc_manager.go @@ -235,7 +235,6 @@ func (m *procManager) DetailedStatusErrorByName(appName string) (string, bool) { // Stop stops the application. func (m *procManager) Stop(name string) error { - m.log.Error("111111111111111111111111111111111111") p, err := m.pop(name) if err != nil { return err From ad32d717576ce8a1c3de411040836c34ae76bf1b Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 18:32:51 +0530 Subject: [PATCH 08/19] Change SetDetailedStatusError to SetError The commit changes SetDetailedStatusError to SetError as it was confusing and SetError is nore consise and accurate. --- internal/vpn/client.go | 10 +++++----- pkg/app/appserver/mock_rpc_ingress_client.go | 4 ++-- pkg/app/appserver/proc.go | 20 ++++++++++---------- pkg/app/appserver/proc_manager.go | 12 ++++++------ pkg/app/appserver/rpc_ingress_client.go | 8 ++++---- pkg/app/appserver/rpc_ingress_gateway.go | 8 ++++---- pkg/app/client.go | 6 +++--- pkg/app/launcher/launcher.go | 4 ++-- pkg/visor/api.go | 10 +++++----- pkg/visor/rpc.go | 16 +++++++++++----- pkg/visor/rpc_client.go | 12 ++++++------ 11 files changed, 58 insertions(+), 52 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 5316c17ded..458fc26ecd 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -201,11 +201,11 @@ func (c *Client) Serve() error { switch err { case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, errHandshakeStatusBadRequest: - c.setAppStatusError(ClientStatus(err.Error())) + c.setAppError(err) return err default: c.setAppStatus(ClientStatusReconnecting) - c.setAppStatusError(ClientStatus(errTimeout.Error())) + c.setAppError(errTimeout) fmt.Println("\nConnection broke, reconnecting...") return fmt.Errorf("dialServeConn: %w", err) } @@ -740,9 +740,9 @@ func (c *Client) setAppStatus(status ClientStatus) { } } -func (c *Client) setAppStatusError(statusErr ClientStatus) { - if err := c.appCl.SetDetailedStatusError(string(statusErr)); err != nil { - fmt.Printf("Failed to set status error %v: %v\n", statusErr, err) +func (c *Client) setAppError(aErr error) { + if err := c.appCl.SetError(aErr.Error()); err != nil { + fmt.Printf("Failed to set error %v: %v\n", aErr, err) } } diff --git a/pkg/app/appserver/mock_rpc_ingress_client.go b/pkg/app/appserver/mock_rpc_ingress_client.go index 3447094700..f4b660a63c 100644 --- a/pkg/app/appserver/mock_rpc_ingress_client.go +++ b/pkg/app/appserver/mock_rpc_ingress_client.go @@ -170,8 +170,8 @@ func (_m *MockRPCIngressClient) SetDetailedStatus(status string) error { return r0 } -// SetDetailedStatusError provides a mock function with given fields: aErr -func (_m *MockRPCIngressClient) SetDetailedStatusError(aErr string) error { +// SetError provides a mock function with given fields: aErr +func (_m *MockRPCIngressClient) SetError(aErr string) error { ret := _m.Called(aErr) var r0 error diff --git a/pkg/app/appserver/proc.go b/pkg/app/appserver/proc.go index 0c00f7149d..d7dd5ca3c7 100644 --- a/pkg/app/appserver/proc.go +++ b/pkg/app/appserver/proc.go @@ -51,10 +51,10 @@ type Proc struct { startTimeMx sync.RWMutex startTime time.Time - statusMx sync.RWMutex - status string - errMx sync.RWMutex - statusErr string + statusMx sync.RWMutex + status string + errMx sync.RWMutex + err string } // NewProc constructs `Proc`. @@ -172,8 +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.SetDetailedStatusError(p.appName, p.statusErr) //nolint:errcheck - _ = p.m.Stop(p.appName) //nolint:errcheck + _ = p.m.SetError(p.appName, p.err) //nolint:errcheck + _ = p.m.Stop(p.appName) //nolint:errcheck }() select { @@ -288,12 +288,12 @@ func (p *Proc) DetailedStatus() string { return p.status } -// SetDetailedStatusError sets proc's detailed status error. -func (p *Proc) SetDetailedStatusError(statusErr string) { +// SetError sets proc's detailed status error. +func (p *Proc) SetError(aErr string) { p.errMx.Lock() defer p.errMx.Unlock() - p.statusErr = statusErr + p.err = aErr } // Error gets proc's error. @@ -301,7 +301,7 @@ func (p *Proc) Error() string { p.errMx.RLock() defer p.errMx.RUnlock() - return p.statusErr + return p.err } // ConnectionSummary sums up the connection stats. diff --git a/pkg/app/appserver/proc_manager.go b/pkg/app/appserver/proc_manager.go index d5aff85640..7e5da3fe2a 100644 --- a/pkg/app/appserver/proc_manager.go +++ b/pkg/app/appserver/proc_manager.go @@ -38,8 +38,8 @@ type ProcManager interface { io.Closer Start(conf appcommon.ProcConfig) (appcommon.ProcID, error) ProcByName(appName string) (*Proc, bool) - SetDetailedStatusError(appName, status string) error - DetailedStatusErrorByName(appName string) (string, 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) @@ -225,7 +225,7 @@ func (m *procManager) ProcByName(appName string) (*Proc, bool) { return proc, ok } -func (m *procManager) DetailedStatusErrorByName(appName string) (string, bool) { +func (m *procManager) ErrorByName(appName string) (string, bool) { m.mx.RLock() defer m.mx.RUnlock() @@ -322,12 +322,12 @@ func (m *procManager) DetailedStatus(appName string) (string, error) { return p.DetailedStatus(), nil } -// SetDetailedStatusError error `statusErr` for app `appName`. -func (m *procManager) SetDetailedStatusError(appName, statusErr string) error { +// SetError error `aErr` for app `appName`. +func (m *procManager) SetError(appName, aErr string) error { m.mx.RLock() defer m.mx.RUnlock() - m.errors[appName] = statusErr + m.errors[appName] = aErr return nil } diff --git a/pkg/app/appserver/rpc_ingress_client.go b/pkg/app/appserver/rpc_ingress_client.go index 8480b16075..3f77fd30b7 100644 --- a/pkg/app/appserver/rpc_ingress_client.go +++ b/pkg/app/appserver/rpc_ingress_client.go @@ -15,7 +15,7 @@ import ( // RPCIngressClient describes RPC interface to communicate with the server. type RPCIngressClient interface { SetDetailedStatus(status string) error - SetDetailedStatusError(aErr string) error + SetError(aErr string) error Dial(remote appnet.Addr) (connID uint16, localPort routing.Port, err error) Listen(local appnet.Addr) (uint16, error) Accept(lisID uint16) (connID uint16, remote appnet.Addr, err error) @@ -47,9 +47,9 @@ func (c *rpcIngressClient) SetDetailedStatus(status string) error { return c.rpc.Call(c.formatMethod("SetDetailedStatus"), &status, nil) } -// SetDetailedStatusError sets detailed status error of an app. -func (c *rpcIngressClient) SetDetailedStatusError(statusErr string) error { - return c.rpc.Call(c.formatMethod("SetDetailedStatusError"), &statusErr, nil) +// SetError sets error of an app. +func (c *rpcIngressClient) SetError(aErr string) error { + return c.rpc.Call(c.formatMethod("SetError"), &aErr, nil) } // Dial sends `Dial` command to the server. diff --git a/pkg/app/appserver/rpc_ingress_gateway.go b/pkg/app/appserver/rpc_ingress_gateway.go index c5ad2c723a..c590a7c042 100644 --- a/pkg/app/appserver/rpc_ingress_gateway.go +++ b/pkg/app/appserver/rpc_ingress_gateway.go @@ -85,11 +85,11 @@ func (r *RPCIngressGateway) SetDetailedStatus(status *string, _ *struct{}) (err return nil } -// SetDetailedStatusError sets error of an app. -func (r *RPCIngressGateway) SetDetailedStatusError(aErr *string, _ *struct{}) (err error) { - defer rpcutil.LogCall(r.log, "SetDetailedStatusError", aErr)(nil, &err) +// SetError sets error of an app. +func (r *RPCIngressGateway) SetError(aErr *string, _ *struct{}) (err error) { + defer rpcutil.LogCall(r.log, "SetError", aErr)(nil, &err) - r.proc.SetDetailedStatusError(*aErr) + r.proc.SetError(*aErr) return nil } diff --git a/pkg/app/client.go b/pkg/app/client.go index 8df262dffb..a170d2e67f 100644 --- a/pkg/app/client.go +++ b/pkg/app/client.go @@ -68,9 +68,9 @@ func (c *Client) SetDetailedStatus(status string) error { return c.rpcC.SetDetailedStatus(status) } -// SetDetailedStatusError sets detailed app status within the visor. -func (c *Client) SetDetailedStatusError(aErr string) error { - return c.rpcC.SetDetailedStatusError(aErr) +// SetError sets app error within the visor. +func (c *Client) SetError(aErr string) error { + return c.rpcC.SetError(aErr) } // Dial dials the remote visor using `remote`. diff --git a/pkg/app/launcher/launcher.go b/pkg/app/launcher/launcher.go index 7d353f66bb..3f7496f353 100644 --- a/pkg/app/launcher/launcher.go +++ b/pkg/app/launcher/launcher.go @@ -173,7 +173,7 @@ func (l *Launcher) AppState(name string) (*AppState, bool) { return nil, false } state := &AppState{AppConfig: ac, Status: AppStatusStopped} - if _, ok := l.procM.DetailedStatusErrorByName(ac.Name); ok { //nolint:errcheck + if _, ok := l.procM.ErrorByName(ac.Name); ok { //nolint:errcheck state.Status = AppStatusErrored } if _, ok := l.procM.ProcByName(ac.Name); ok { //nolint:errcheck @@ -190,7 +190,7 @@ func (l *Launcher) AppStates() []*AppState { var states []*AppState for _, app := range l.apps { state := &AppState{AppConfig: app, Status: AppStatusStopped} - if err, ok := l.procM.DetailedStatusErrorByName(app.Name); ok { + if err, ok := l.procM.ErrorByName(app.Name); ok { state.DetailedStatus = err state.Status = AppStatusErrored } diff --git a/pkg/visor/api.go b/pkg/visor/api.go index ffaf1a3328..5889970f8f 100644 --- a/pkg/visor/api.go +++ b/pkg/visor/api.go @@ -40,7 +40,7 @@ type API interface { StartApp(appName string) error StopApp(appName string) error SetAppDetailedStatus(appName, state string) error - SetAppDetailedStatusError(appName, stateErr string) error + SetAppError(appName, stateErr string) error RestartApp(appName string) error SetAutoStart(appName string, autostart bool) error SetAppPassword(appName, password string) error @@ -350,15 +350,15 @@ func (v *Visor) SetAppDetailedStatus(appName, status string) error { return nil } -// SetAppDetailedStatusError implements API. -func (v *Visor) SetAppDetailedStatusError(appName, aErr string) error { +// SetAppError implements API. +func (v *Visor) SetAppError(appName, aErr string) error { proc, ok := v.procM.ProcByName(appName) if !ok { return ErrAppProcNotRunning } - v.log.Infof("Setting app detailed status error %v for app %v", aErr, appName) - proc.SetDetailedStatusError(aErr) + v.log.Infof("Setting error %v for app %v", aErr, appName) + proc.SetError(aErr) return nil } diff --git a/pkg/visor/rpc.go b/pkg/visor/rpc.go index 69a219dec6..a46065cba9 100644 --- a/pkg/visor/rpc.go +++ b/pkg/visor/rpc.go @@ -170,12 +170,18 @@ func (r *RPC) Overview(_ *struct{}, out *Overview) (err error) { <<< APP MANAGEMENT >>> */ -// SetAppStatusIn is input for SetAppDetailedStatus and SetAppDetailedStatusError. +// SetAppStatusIn is input for SetAppDetailedStatus. type SetAppStatusIn struct { AppName string Status string } +// SetAppStatusIn is input for SetAppError. +type SetAppErrorIn struct { + AppName string + Err string +} + // SetAppDetailedStatus sets app's detailed status. func (r *RPC) SetAppDetailedStatus(in *SetAppStatusIn, _ *struct{}) (err error) { defer rpcutil.LogCall(r.log, "SetAppDetailedStatus", in)(nil, &err) @@ -183,11 +189,11 @@ func (r *RPC) SetAppDetailedStatus(in *SetAppStatusIn, _ *struct{}) (err error) return r.visor.SetAppDetailedStatus(in.AppName, in.Status) } -// SetAppDetailedStatusError sets app's detailed status error. -func (r *RPC) SetAppDetailedStatusError(in *SetAppStatusIn, _ *struct{}) (err error) { - defer rpcutil.LogCall(r.log, "SetAppDetailedStatusError", in)(nil, &err) +// SetAppError sets app's error. +func (r *RPC) SetAppError(in *SetAppErrorIn, _ *struct{}) (err error) { + defer rpcutil.LogCall(r.log, "SetAppError", in)(nil, &err) - return r.visor.SetAppDetailedStatusError(in.AppName, in.Status) + return r.visor.SetAppError(in.AppName, in.Err) } // Apps returns list of Apps registered on the Visor. diff --git a/pkg/visor/rpc_client.go b/pkg/visor/rpc_client.go index 4e4b530ddd..ad6f61f1f0 100644 --- a/pkg/visor/rpc_client.go +++ b/pkg/visor/rpc_client.go @@ -146,11 +146,11 @@ func (rc *rpcClient) SetAppDetailedStatus(appName, status string) error { }, &struct{}{}) } -// SetAppDetailedStatusError sets app's detailed status error. -func (rc *rpcClient) SetAppDetailedStatusError(appName, statusErr string) error { - return rc.Call("SetAppDetailedStatusError", &SetAppStatusIn{ +// SetAppError sets app's error. +func (rc *rpcClient) SetAppError(appName, aErr string) error { + return rc.Call("SetAppError", &SetAppErrorIn{ AppName: appName, - Status: statusErr, + Err: aErr, }, &struct{}{}) } @@ -686,8 +686,8 @@ func (mc *mockRPCClient) SetAppDetailedStatus(appName, status string) error { }) } -// SetAppDetailedStatusError sets app's detailed state error. -func (mc *mockRPCClient) SetAppDetailedStatusError(appName, aErr string) error { +// SetAppError sets app's error. +func (mc *mockRPCClient) SetAppError(appName, aErr string) error { return mc.do(true, func() error { for _, a := range mc.o.Apps { if a.Name == appName { From 7b3743002578ca7620015b0cd01966374cbaae36 Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 18:46:51 +0530 Subject: [PATCH 09/19] Fix func comment --- pkg/app/appserver/proc_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/appserver/proc_manager.go b/pkg/app/appserver/proc_manager.go index 7e5da3fe2a..c4abaf2255 100644 --- a/pkg/app/appserver/proc_manager.go +++ b/pkg/app/appserver/proc_manager.go @@ -322,7 +322,7 @@ func (m *procManager) DetailedStatus(appName string) (string, error) { return p.DetailedStatus(), nil } -// SetError error `aErr` for app `appName`. +// SetError sets error `aErr` for app `appName`. func (m *procManager) SetError(appName, aErr string) error { m.mx.RLock() defer m.mx.RUnlock() From 84b30245d8a72af988d7eb0ff8312deb086c90bc Mon Sep 17 00:00:00 2001 From: ersonp Date: Mon, 4 Oct 2021 19:47:33 +0530 Subject: [PATCH 10/19] Fix linting error --- pkg/visor/rpc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/visor/rpc.go b/pkg/visor/rpc.go index a46065cba9..4256cebe87 100644 --- a/pkg/visor/rpc.go +++ b/pkg/visor/rpc.go @@ -176,7 +176,7 @@ type SetAppStatusIn struct { Status string } -// SetAppStatusIn is input for SetAppError. +// SetAppErrorIn is input for SetAppError. type SetAppErrorIn struct { AppName string Err string From 2db92fb005540378efde6aa39df5191970026810 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 12:09:39 +0530 Subject: [PATCH 11/19] Add transport check The commit contains a transport check in vpn-client. Before dialing the routes the available transports to the server are checked. If there is none then we send a ErrTransportNotFound. ErrTransportNotFound is whitelisted in the dail retrier. --- internal/vpn/client.go | 8 +++++++- pkg/router/router.go | 18 ++++++++++++++++++ pkg/transport/manager.go | 8 +++++--- pkg/visor/api.go | 6 +++++- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 458fc26ecd..aa34acbfa2 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -18,6 +18,7 @@ import ( "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appnet" + "github.com/skycoin/skywire/pkg/routefinder/rfclient" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" skynetutil "github.com/skycoin/skywire/pkg/util/netutil" @@ -115,7 +116,7 @@ func NewClient(cfg ClientConfig, appCl *app.Client) (*Client, error) { ) log := logrus.New() - r := netutil.NewRetrier(log, serverDialInitBO, serverDialMaxBO, 0, 1) + r := netutil.NewRetrier(log, serverDialInitBO, serverDialMaxBO, 0, 1).WithErrWhitelist(rfclient.ErrTransportNotFound) defaultGateway, err := DefaultNetworkGateway() if err != nil { @@ -719,6 +720,11 @@ func (c *Client) dialServer(appCl *app.Client, pk cipher.PubKey) (net.Conn, erro return nil } + // WithErrWhitelist only recognizes the error if it's the same variable + if err.Error() == rfclient.ErrTransportNotFound.Error() { + return rfclient.ErrTransportNotFound + } + return err }) if err != nil { diff --git a/pkg/router/router.go b/pkg/router/router.go index 24feec44f6..8a3274a22c 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -218,6 +218,11 @@ func (r *router) DialRoutes( lPK := r.conf.PubKey forwardDesc := routing.NewRouteDescriptor(lPK, rPK, lPort, rPort) + ok := r.checkIfTransportAvailabel(rPK) + if !ok { + return nil, rfclient.ErrTransportNotFound + } + forwardPath, reversePath, err := r.fetchBestRoutes(lPK, rPK, opts) if err != nil { return nil, fmt.Errorf("route finder: %w", err) @@ -1030,3 +1035,16 @@ func (r *router) removeRouteGroupOfRule(rule routing.Rule) { } log.Debug("Noise route group closed.") } + +func (r *router) checkIfTransportAvailabel(rPK cipher.PubKey) (ok bool) { + networks := r.tm.Networks() + + for _, network := range networks { + _, err := r.tm.GetTransport(rPK, network) + if err == nil { + ok = true + } + } + + return ok +} diff --git a/pkg/transport/manager.go b/pkg/transport/manager.go index 1acff54d24..c5e1afd871 100644 --- a/pkg/transport/manager.go +++ b/pkg/transport/manager.go @@ -248,10 +248,12 @@ func (tm *Manager) cleanupTransports(ctx context.Context) { } // Networks returns all the network types contained within the TransportManager. -func (tm *Manager) Networks() []string { - var nets []string +func (tm *Manager) Networks() []network.Type { + tm.mx.Lock() + defer tm.mx.Unlock() + var nets []network.Type for netType := range tm.netClients { - nets = append(nets, string(netType)) + nets = append(nets, netType) } return nets } diff --git a/pkg/visor/api.go b/pkg/visor/api.go index 5889970f8f..2de6e6c2ca 100644 --- a/pkg/visor/api.go +++ b/pkg/visor/api.go @@ -525,7 +525,11 @@ func (v *Visor) GetAppConnectionsSummary(appName string) ([]appserver.Connection // TransportTypes implements API. func (v *Visor) TransportTypes() ([]string, error) { - return v.tpM.Networks(), nil + var types []string + for _, netType := range v.tpM.Networks() { + types = append(types, string(netType)) + } + return types, nil } // Transports implements API. From b437f920b82aca5565e20dee6a4b7511a4d0bf22 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 13:26:12 +0530 Subject: [PATCH 12/19] Requested changes The commit contains updated error for unknown code, aErr changed to appErr, removed switch statements and whitelisted ErrTransportNotFound in main retrier. --- internal/vpn/client.go | 40 +++++++++----------- internal/vpn/handshake_status.go | 2 +- pkg/app/appserver/mock_rpc_ingress_client.go | 8 ++-- pkg/app/appserver/proc.go | 4 +- pkg/app/appserver/proc_manager.go | 6 +-- pkg/app/appserver/rpc_ingress_client.go | 6 +-- pkg/app/appserver/rpc_ingress_gateway.go | 6 +-- pkg/app/client.go | 4 +- pkg/visor/api.go | 6 +-- pkg/visor/rpc_client.go | 8 ++-- 10 files changed, 43 insertions(+), 47 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index aa34acbfa2..0e43c53cb6 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -191,7 +191,8 @@ func (c *Client) Serve() error { r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor). WithErrWhitelist(errHandshakeStatusForbidden).WithErrWhitelist(errHandshakeStatusInternalError). - WithErrWhitelist(errHandshakeNoFreeIPs).WithErrWhitelist(errHandshakeStatusBadRequest) + WithErrWhitelist(errHandshakeNoFreeIPs).WithErrWhitelist(errHandshakeStatusBadRequest). + WithErrWhitelist(rfclient.ErrTransportNotFound) err := r.Do(context.Background(), func() error { if c.isClosed() { @@ -201,7 +202,7 @@ func (c *Client) Serve() error { if err := c.dialServeConn(); err != nil { switch err { case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest: + errHandshakeStatusBadRequest, rfclient.ErrTransportNotFound: c.setAppError(err) return err default: @@ -338,12 +339,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 { - switch err { - case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest: - return err - } - 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()) @@ -446,7 +443,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()) @@ -462,12 +460,8 @@ func (c *Client) dialServeConn() error { } if err := c.serveConn(conn); err != nil { - switch err { - case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest: - return err - } - return fmt.Errorf("error serving app conn: %w", err) + fmt.Printf("error serving app conn: %s", err) + return err } return nil @@ -685,7 +679,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, HandshakeStatusForbidden.getError() + return nil, nil, sHello.Status.getError() } return sHello.TUNIP, sHello.TUNGateway, nil @@ -720,9 +714,11 @@ func (c *Client) dialServer(appCl *app.Client, pk cipher.PubKey) (net.Conn, erro return nil } - // WithErrWhitelist only recognizes the error if it's the same variable - if err.Error() == rfclient.ErrTransportNotFound.Error() { - return rfclient.ErrTransportNotFound + if err != nil { + // WithErrWhitelist only recognizes the error if it's the same variable + if err.Error() == rfclient.ErrTransportNotFound.Error() { + return rfclient.ErrTransportNotFound + } } return err @@ -746,9 +742,9 @@ func (c *Client) setAppStatus(status ClientStatus) { } } -func (c *Client) setAppError(aErr error) { - if err := c.appCl.SetError(aErr.Error()); err != nil { - fmt.Printf("Failed to set error %v: %v\n", aErr, err) +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) } } diff --git a/internal/vpn/handshake_status.go b/internal/vpn/handshake_status.go index 5463e2bde0..ec44ab2c2b 100644 --- a/internal/vpn/handshake_status.go +++ b/internal/vpn/handshake_status.go @@ -48,6 +48,6 @@ func (hs HandshakeStatus) getError() error { case HandshakeStatusForbidden: return errHandshakeStatusForbidden default: - return errors.New("Unknown code") + return errors.New("Unknown error code") } } diff --git a/pkg/app/appserver/mock_rpc_ingress_client.go b/pkg/app/appserver/mock_rpc_ingress_client.go index f4b660a63c..7a17aa41e0 100644 --- a/pkg/app/appserver/mock_rpc_ingress_client.go +++ b/pkg/app/appserver/mock_rpc_ingress_client.go @@ -170,13 +170,13 @@ func (_m *MockRPCIngressClient) SetDetailedStatus(status string) error { return r0 } -// SetError provides a mock function with given fields: aErr -func (_m *MockRPCIngressClient) SetError(aErr string) error { - ret := _m.Called(aErr) +// SetError provides a mock function with given fields: appErr +func (_m *MockRPCIngressClient) SetError(appErr string) error { + ret := _m.Called(appErr) var r0 error if rf, ok := ret.Get(0).(func(string) error); ok { - r0 = rf(aErr) + r0 = rf(appErr) } else { r0 = ret.Error(0) } diff --git a/pkg/app/appserver/proc.go b/pkg/app/appserver/proc.go index d7dd5ca3c7..c57a01af65 100644 --- a/pkg/app/appserver/proc.go +++ b/pkg/app/appserver/proc.go @@ -289,11 +289,11 @@ func (p *Proc) DetailedStatus() string { } // SetError sets proc's detailed status error. -func (p *Proc) SetError(aErr string) { +func (p *Proc) SetError(appErr string) { p.errMx.Lock() defer p.errMx.Unlock() - p.err = aErr + p.err = appErr } // Error gets proc's error. diff --git a/pkg/app/appserver/proc_manager.go b/pkg/app/appserver/proc_manager.go index c4abaf2255..ad281da3f3 100644 --- a/pkg/app/appserver/proc_manager.go +++ b/pkg/app/appserver/proc_manager.go @@ -322,12 +322,12 @@ func (m *procManager) DetailedStatus(appName string) (string, error) { return p.DetailedStatus(), nil } -// SetError sets error `aErr` for app `appName`. -func (m *procManager) SetError(appName, aErr string) error { +// SetError sets error `appErr` for app `appName`. +func (m *procManager) SetError(appName, appErr string) error { m.mx.RLock() defer m.mx.RUnlock() - m.errors[appName] = aErr + m.errors[appName] = appErr return nil } diff --git a/pkg/app/appserver/rpc_ingress_client.go b/pkg/app/appserver/rpc_ingress_client.go index 3f77fd30b7..4d98297765 100644 --- a/pkg/app/appserver/rpc_ingress_client.go +++ b/pkg/app/appserver/rpc_ingress_client.go @@ -15,7 +15,7 @@ import ( // RPCIngressClient describes RPC interface to communicate with the server. type RPCIngressClient interface { SetDetailedStatus(status string) error - SetError(aErr string) error + SetError(appErr string) error Dial(remote appnet.Addr) (connID uint16, localPort routing.Port, err error) Listen(local appnet.Addr) (uint16, error) Accept(lisID uint16) (connID uint16, remote appnet.Addr, err error) @@ -48,8 +48,8 @@ func (c *rpcIngressClient) SetDetailedStatus(status string) error { } // SetError sets error of an app. -func (c *rpcIngressClient) SetError(aErr string) error { - return c.rpc.Call(c.formatMethod("SetError"), &aErr, nil) +func (c *rpcIngressClient) SetError(appErr string) error { + return c.rpc.Call(c.formatMethod("SetError"), &appErr, nil) } // Dial sends `Dial` command to the server. diff --git a/pkg/app/appserver/rpc_ingress_gateway.go b/pkg/app/appserver/rpc_ingress_gateway.go index c590a7c042..c95e584163 100644 --- a/pkg/app/appserver/rpc_ingress_gateway.go +++ b/pkg/app/appserver/rpc_ingress_gateway.go @@ -86,10 +86,10 @@ func (r *RPCIngressGateway) SetDetailedStatus(status *string, _ *struct{}) (err } // SetError sets error of an app. -func (r *RPCIngressGateway) SetError(aErr *string, _ *struct{}) (err error) { - defer rpcutil.LogCall(r.log, "SetError", aErr)(nil, &err) +func (r *RPCIngressGateway) SetError(appErr *string, _ *struct{}) (err error) { + defer rpcutil.LogCall(r.log, "SetError", appErr)(nil, &err) - r.proc.SetError(*aErr) + r.proc.SetError(*appErr) return nil } diff --git a/pkg/app/client.go b/pkg/app/client.go index a170d2e67f..6ce8756c43 100644 --- a/pkg/app/client.go +++ b/pkg/app/client.go @@ -69,8 +69,8 @@ func (c *Client) SetDetailedStatus(status string) error { } // SetError sets app error within the visor. -func (c *Client) SetError(aErr string) error { - return c.rpcC.SetError(aErr) +func (c *Client) SetError(appErr string) error { + return c.rpcC.SetError(appErr) } // Dial dials the remote visor using `remote`. diff --git a/pkg/visor/api.go b/pkg/visor/api.go index 2de6e6c2ca..3099f1faca 100644 --- a/pkg/visor/api.go +++ b/pkg/visor/api.go @@ -351,14 +351,14 @@ func (v *Visor) SetAppDetailedStatus(appName, status string) error { } // SetAppError implements API. -func (v *Visor) SetAppError(appName, aErr string) error { +func (v *Visor) SetAppError(appName, appErr string) error { proc, ok := v.procM.ProcByName(appName) if !ok { return ErrAppProcNotRunning } - v.log.Infof("Setting error %v for app %v", aErr, appName) - proc.SetError(aErr) + v.log.Infof("Setting error %v for app %v", appErr, appName) + proc.SetError(appErr) return nil } diff --git a/pkg/visor/rpc_client.go b/pkg/visor/rpc_client.go index ad6f61f1f0..51cc9fce12 100644 --- a/pkg/visor/rpc_client.go +++ b/pkg/visor/rpc_client.go @@ -147,10 +147,10 @@ func (rc *rpcClient) SetAppDetailedStatus(appName, status string) error { } // SetAppError sets app's error. -func (rc *rpcClient) SetAppError(appName, aErr string) error { +func (rc *rpcClient) SetAppError(appName, appErr string) error { return rc.Call("SetAppError", &SetAppErrorIn{ AppName: appName, - Err: aErr, + Err: appErr, }, &struct{}{}) } @@ -687,11 +687,11 @@ func (mc *mockRPCClient) SetAppDetailedStatus(appName, status string) error { } // SetAppError sets app's error. -func (mc *mockRPCClient) SetAppError(appName, aErr string) error { +func (mc *mockRPCClient) SetAppError(appName, appErr string) error { return mc.do(true, func() error { for _, a := range mc.o.Apps { if a.Name == appName { - a.DetailedStatus = aErr + a.DetailedStatus = appErr return nil } } From abf6e3537c7539204a93681c568a1c9b8781fc15 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 13:34:01 +0530 Subject: [PATCH 13/19] Make format --- pkg/app/appserver/mock_rpc_ingress_client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/app/appserver/mock_rpc_ingress_client.go b/pkg/app/appserver/mock_rpc_ingress_client.go index 7a17aa41e0..7293180e01 100644 --- a/pkg/app/appserver/mock_rpc_ingress_client.go +++ b/pkg/app/appserver/mock_rpc_ingress_client.go @@ -3,12 +3,12 @@ package appserver import ( - appnet "github.com/skycoin/skywire/pkg/app/appnet" + time "time" + mock "github.com/stretchr/testify/mock" + appnet "github.com/skycoin/skywire/pkg/app/appnet" routing "github.com/skycoin/skywire/pkg/routing" - - time "time" ) // MockRPCIngressClient is an autogenerated mock type for the RPCIngressClient type From bb2aa6140d32b96db915c537aad913324757371b Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 16:20:56 +0530 Subject: [PATCH 14/19] Remove dialServer retrier The commit removes the retrier in dialServer as it's redundant and the same job can be done by the main retrier. --- internal/vpn/client.go | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 0e43c53cb6..5aa9f2b1fb 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -35,7 +35,6 @@ type Client struct { log *logrus.Logger cfg ClientConfig appCl *app.Client - r *netutil.Retrier directIPSMu sync.Mutex directIPs []net.IP defaultGateway net.IP @@ -110,13 +109,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).WithErrWhitelist(rfclient.ErrTransportNotFound) defaultGateway, err := DefaultNetworkGateway() if err != nil { @@ -129,7 +122,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{}), @@ -700,30 +692,18 @@ 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 - } - - if err != nil { - // WithErrWhitelist only recognizes the error if it's the same variable - if err.Error() == rfclient.ErrTransportNotFound.Error() { - return rfclient.ErrTransportNotFound - } - } - - return err + var err error + conn, err = appCl.Dial(appnet.Addr{ + Net: netType, + PubKey: pk, + Port: vpnPort, }) + if err != nil { + // WithErrWhitelist only recognizes the error if it's the same variable + if err.Error() == rfclient.ErrTransportNotFound.Error() { + return nil, rfclient.ErrTransportNotFound + } return nil, err } From 9eab0171c68c78ca66d9c2c588e4a4f55894cce1 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 18:12:43 +0530 Subject: [PATCH 15/19] Add RPCErr The commit adds RPCErr struct that is used to preserve the type of the error from RPC. Currelty only the err of Dial() is wrapped with it. --- internal/vpn/client.go | 13 +++++++------ pkg/app/appserver/rpc_ingress_client.go | 10 +++++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 5aa9f2b1fb..444cf8e5e6 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -18,6 +18,7 @@ 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/routing" "github.com/skycoin/skywire/pkg/skyenv" @@ -181,10 +182,14 @@ func (c *Client) Serve() error { c.setAppStatus(ClientStatusConnecting) + errTransportNotFound := appserver.RPCErr{ + Err: rfclient.ErrTransportNotFound.Error(), + } + r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor). WithErrWhitelist(errHandshakeStatusForbidden).WithErrWhitelist(errHandshakeStatusInternalError). WithErrWhitelist(errHandshakeNoFreeIPs).WithErrWhitelist(errHandshakeStatusBadRequest). - WithErrWhitelist(rfclient.ErrTransportNotFound) + WithErrWhitelist(errTransportNotFound) err := r.Do(context.Background(), func() error { if c.isClosed() { @@ -194,7 +199,7 @@ func (c *Client) Serve() error { if err := c.dialServeConn(); err != nil { switch err { case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest, rfclient.ErrTransportNotFound: + errHandshakeStatusBadRequest, errTransportNotFound: c.setAppError(err) return err default: @@ -700,10 +705,6 @@ func (c *Client) dialServer(appCl *app.Client, pk cipher.PubKey) (net.Conn, erro }) if err != nil { - // WithErrWhitelist only recognizes the error if it's the same variable - if err.Error() == rfclient.ErrTransportNotFound.Error() { - return nil, rfclient.ErrTransportNotFound - } return nil, err } diff --git a/pkg/app/appserver/rpc_ingress_client.go b/pkg/app/appserver/rpc_ingress_client.go index 4d98297765..3d9bfd593b 100644 --- a/pkg/app/appserver/rpc_ingress_client.go +++ b/pkg/app/appserver/rpc_ingress_client.go @@ -52,11 +52,19 @@ func (c *rpcIngressClient) SetError(appErr string) error { return c.rpc.Call(c.formatMethod("SetError"), &appErr, nil) } +type RPCErr struct { + Err string +} + +func (e RPCErr) Error() string { + return e.Err +} + // Dial sends `Dial` command to the server. func (c *rpcIngressClient) Dial(remote appnet.Addr) (connID uint16, localPort routing.Port, err error) { var resp DialResp if err := c.rpc.Call(c.formatMethod("Dial"), &remote, &resp); err != nil { - return 0, 0, err + return 0, 0, RPCErr{err.Error()} } return resp.ConnID, resp.LocalPort, nil From e1258baa004a5792af377115903776374a95b754 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 19:29:37 +0530 Subject: [PATCH 16/19] Fix linting --- pkg/app/appserver/rpc_ingress_client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/app/appserver/rpc_ingress_client.go b/pkg/app/appserver/rpc_ingress_client.go index 3d9bfd593b..acfe42d021 100644 --- a/pkg/app/appserver/rpc_ingress_client.go +++ b/pkg/app/appserver/rpc_ingress_client.go @@ -52,6 +52,7 @@ func (c *rpcIngressClient) SetError(appErr string) error { return c.rpc.Call(c.formatMethod("SetError"), &appErr, nil) } +// RPCErr is used to preserve the type of the errors we return via RPC type RPCErr struct { Err string } From 0b6d34a8c188425406a50ccb86c4e4fd05edf9e4 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 5 Oct 2021 23:27:08 +0530 Subject: [PATCH 17/19] Requested changes --- internal/vpn/client.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 444cf8e5e6..a8066dc57a 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -187,9 +187,8 @@ func (c *Client) Serve() error { } r := netutil.NewRetrier(c.log, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor). - WithErrWhitelist(errHandshakeStatusForbidden).WithErrWhitelist(errHandshakeStatusInternalError). - WithErrWhitelist(errHandshakeNoFreeIPs).WithErrWhitelist(errHandshakeStatusBadRequest). - WithErrWhitelist(errTransportNotFound) + WithErrWhitelist(errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, + errHandshakeStatusBadRequest, errTransportNotFound) err := r.Do(context.Background(), func() error { if c.isClosed() { From c7d09dcc457c1f8ab385c7b7e99ea99d7c5afd4e Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 6 Oct 2021 11:30:07 +0530 Subject: [PATCH 18/19] Requested changes The commit updates the checkIfTransportAvalailable to check if any transports are established for the visor and not if there is a transport between local and remote visor. checkIfTransportAvalailable also sends a new error ErrNoTransportFound if no transports are established for the visor. Both ErrTransportNotFound and ErrNoTransportFound are whitelisted as ErrTransportNotFound is received from router whenever it is unsuccessful at creating a route to the remote visor.. --- internal/vpn/client.go | 9 +++++++-- pkg/router/router.go | 22 ++++++++++------------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index a8066dc57a..46dde8bfc4 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -20,6 +20,7 @@ import ( "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" @@ -182,13 +183,17 @@ func (c *Client) Serve() error { c.setAppStatus(ClientStatusConnecting) + 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, errTransportNotFound) + errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound) err := r.Do(context.Background(), func() error { if c.isClosed() { @@ -198,7 +203,7 @@ func (c *Client) Serve() error { if err := c.dialServeConn(); err != nil { switch err { case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest, errTransportNotFound: + errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound: c.setAppError(err) return err default: diff --git a/pkg/router/router.go b/pkg/router/router.go index 8a3274a22c..08fad3857d 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -47,6 +47,9 @@ var ( // ErrRemoteEmptyPK occurs when the specified remote public key is empty. ErrRemoteEmptyPK = errors.New("empty remote public key") + + // ErrNoTransportFound is returned when not even one transport is found. + ErrNoTransportFound = errors.New("no transport found") ) // Config configures Router. @@ -218,9 +221,9 @@ func (r *router) DialRoutes( lPK := r.conf.PubKey forwardDesc := routing.NewRouteDescriptor(lPK, rPK, lPort, rPort) - ok := r.checkIfTransportAvailabel(rPK) + ok := r.checkIfTransportAvalailable() if !ok { - return nil, rfclient.ErrTransportNotFound + return nil, ErrNoTransportFound } forwardPath, reversePath, err := r.fetchBestRoutes(lPK, rPK, opts) @@ -1036,15 +1039,10 @@ func (r *router) removeRouteGroupOfRule(rule routing.Rule) { log.Debug("Noise route group closed.") } -func (r *router) checkIfTransportAvailabel(rPK cipher.PubKey) (ok bool) { - networks := r.tm.Networks() - - for _, network := range networks { - _, err := r.tm.GetTransport(rPK, network) - if err == nil { - ok = true - } - } - +func (r *router) checkIfTransportAvalailable() (ok bool) { + r.tm.WalkTransports(func(tp *transport.ManagedTransport) bool { + ok = true + return true + }) return ok } From a7349db3dc68f29f84419e7295524bb31d76f899 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 6 Oct 2021 18:26:20 +0530 Subject: [PATCH 19/19] Fix datarace The commit changes the lock in SetError() from RLock to Lock. --- pkg/app/appserver/proc_manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/app/appserver/proc_manager.go b/pkg/app/appserver/proc_manager.go index ad281da3f3..dc359b97d4 100644 --- a/pkg/app/appserver/proc_manager.go +++ b/pkg/app/appserver/proc_manager.go @@ -324,8 +324,8 @@ func (m *procManager) DetailedStatus(appName string) (string, error) { // SetError sets error `appErr` for app `appName`. func (m *procManager) SetError(appName, appErr string) error { - m.mx.RLock() - defer m.mx.RUnlock() + m.mx.Lock() + defer m.mx.Unlock() m.errors[appName] = appErr