diff options
| author | Aleksandar Pesic <peske.nis@gmail.com> | 2022-12-04 21:48:20 +0100 |
|---|---|---|
| committer | jwijenbergh <jeroenwijenbergh@protonmail.com> | 2022-12-12 13:26:51 +0100 |
| commit | 3ac1d35257b56cca92ad0eb7f4d18abb366cf105 (patch) | |
| tree | 432db14d1f92a252518f371be420fa0d3ef044c8 /client | |
| parent | 37bca013bd4405548b274ac473acf959ad661ee6 (diff) | |
simplify error handling
fixes #6
Signed-off-by: Aleksandar Pesic <peske.nis@gmail.com>
Diffstat (limited to 'client')
| -rw-r--r-- | client/client.go | 155 | ||||
| -rw-r--r-- | client/client_test.go | 65 | ||||
| -rw-r--r-- | client/fsm.go | 235 | ||||
| -rw-r--r-- | client/server.go | 651 |
4 files changed, 491 insertions, 615 deletions
diff --git a/client/client.go b/client/client.go index 1229a20..a11bf4e 100644 --- a/client/client.go +++ b/client/client.go @@ -1,4 +1,4 @@ -// package client implements the public interface for creating eduVPN/Let's Connect! clients +// Package client implements the public interface for creating eduVPN/Let's Connect! clients package client import ( @@ -11,6 +11,7 @@ import ( "github.com/eduvpn/eduvpn-common/internal/server" "github.com/eduvpn/eduvpn-common/internal/util" "github.com/eduvpn/eduvpn-common/types" + "github.com/go-errors/errors" ) type ( @@ -19,19 +20,14 @@ type ( ServerBase = server.Base ) -// This wraps the error, logs it and then returns the wrapped error. -func (client *Client) handleError(message string, err error) error { - if err != nil { - // Logs the error with the same level/verbosity as the error - client.Logger.Inherit(message, err) - return types.NewWrappedError(message, err) - } - return nil +func (c *Client) logError(err error) { + // Logs the error with the same level/verbosity as the error + c.Logger.Inherit(err) } -func (client Client) isLetsConnect() bool { +func (c *Client) isLetsConnect() bool { // see https://git.sr.ht/~fkooman/vpn-user-portal/tree/v3/item/src/OAuth/ClientDb.php - return strings.HasPrefix(client.Name, "org.letsconnect-vpn.app") + return strings.HasPrefix(c.Name, "org.letsconnect-vpn.app") } // Client is the main struct for the VPN client. @@ -71,100 +67,98 @@ type Client struct { // - debug: whether or not we want to enable debugging // // It returns an error if initialization failed, for example when discovery cannot be obtained and when there are no servers. -func (client *Client) Register( +func (c *Client) Register( name string, directory string, language string, stateCallback func(FSMStateID, FSMStateID, interface{}) bool, debug bool, -) error { - errorMessage := "failed to register with the GO library" - if !client.InFSMState(StateDeregistered) { - return client.handleError( - errorMessage, - FSMDeregisteredError{}.CustomError(), - ) +) (err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() + + if !c.InFSMState(StateDeregistered) { + return errors.Errorf("fsm attempt to register while in '%v'", c.FSM.Current) } - client.Name = name + + c.Name = name // TODO: Verify language setting? - client.Language = language + c.Language = language // Initialize the logger - logLevel := log.LevelWarning + lvl := log.LevelWarning if debug { - logLevel = log.LevelDebug + lvl = log.LevelDebug } - loggerErr := client.Logger.Init(logLevel, directory) - if loggerErr != nil { - return client.handleError(errorMessage, loggerErr) + if err = c.Logger.Init(lvl, directory); err != nil { + return err } // Initialize the FSM - client.FSM = newFSM(stateCallback, directory, debug) + c.FSM = newFSM(stateCallback, directory, debug) // By default we support wireguard - client.SupportsWireguard = true + c.SupportsWireguard = true // Debug only if given - client.Debug = debug + c.Debug = debug // Initialize the Config - client.Config.Init(directory, "state") + c.Config.Init(directory, "state") // Try to load the previous configuration - if client.Config.Load(&client) != nil { + if c.Config.Load(&c) != nil { // This error can be safely ignored, as when the config does not load, the struct will not be filled - client.Logger.Infof("Previous configuration not found") + c.Logger.Infof("Previous configuration not found") } // Go to the No Server state with the saved servers after we're done - defer client.FSM.GoTransitionWithData(StateNoServer, client.Servers) + defer c.FSM.GoTransitionWithData(StateNoServer, c.Servers) // Let's Connect! doesn't care about discovery - if client.isLetsConnect() { + if c.isLetsConnect() { return nil } // Check if we are able to fetch discovery, and log if something went wrong - _, discoServersErr := client.DiscoServers() - if discoServersErr != nil { - client.Logger.Warningf("Failed to get discovery servers: %v", discoServersErr) + if _, err := c.DiscoServers(); err != nil { + c.Logger.Warningf("Failed to get discovery servers: %v", err) } - _, discoOrgsErr := client.DiscoOrganizations() - if discoOrgsErr != nil { - client.Logger.Warningf("Failed to get discovery organizations: %v", discoOrgsErr) + + if _, err := c.DiscoOrganizations(); err != nil { + c.Logger.Warningf("Failed to get discovery organizations: %v", err) } return nil } // Deregister 'deregisters' the client, meaning saving the log file and the config and emptying out the client struct. -func (client *Client) Deregister() { +func (c *Client) Deregister() { // Close the log file - client.Logger.Close() + _ = c.Logger.Close() // Save the config - saveErr := client.Config.Save(&client) - if saveErr != nil { - client.Logger.Infof("failed saving configuration, error: %s", types.ErrorTraceback(saveErr)) + if err := c.Config.Save(&c); err != nil { + c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } // Empty out the state - *client = Client{} + *c = Client{} } // askProfile asks the user for a profile by moving the FSM to the ASK_PROFILE state. -func (client *Client) askProfile(chosenServer server.Server) error { - errorMessage := "failed asking for profiles" - profiles, profilesErr := server.ValidProfiles(chosenServer, client.SupportsWireguard) - if profilesErr != nil { - return types.NewWrappedError(errorMessage, profilesErr) +func (c *Client) askProfile(srv server.Server) error { + ps, err := server.ValidProfiles(srv, c.SupportsWireguard) + if err != nil { + return err } - goTransitionErr := client.FSM.GoTransitionRequired(StateAskProfile, profiles) - if goTransitionErr != nil { - return types.NewWrappedError(errorMessage, goTransitionErr) + if err = c.FSM.GoTransitionRequired(StateAskProfile, ps); err != nil { + return err } return nil } @@ -173,52 +167,41 @@ func (client *Client) askProfile(chosenServer server.Server) error { // If the list cannot be retrieved an error is returned. // If this is the case then a previous version of the list is returned if there is any. // This takes into account the frequency of updates, see: https://github.com/eduvpn/documentation/blob/v3/SERVER_DISCOVERY.md#organization-list. -func (client *Client) DiscoOrganizations() (*types.DiscoveryOrganizations, error) { - errorMessage := "failed getting discovery organizations list" +func (c *Client) DiscoOrganizations() (orgs *types.DiscoveryOrganizations, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() + // Not supported with Let's Connect! - if client.isLetsConnect() { - return nil, client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + return nil, errors.Errorf("discovery with Let's Connect is not supported") } - orgs, orgsErr := client.Discovery.Organizations() - if orgsErr != nil { - return nil, client.handleError( - errorMessage, - orgsErr, - ) - } - return orgs, nil + return c.Discovery.Organizations() } // DiscoServers gets the servers list from the discovery server // If the list cannot be retrieved an error is returned. // If this is the case then a previous version of the list is returned if there is any. // This takes into account the frequency of updates, see: https://github.com/eduvpn/documentation/blob/v3/SERVER_DISCOVERY.md#server-list. -func (client *Client) DiscoServers() (*types.DiscoveryServers, error) { - errorMessage := "failed getting discovery servers list" +func (c *Client) DiscoServers() (dss *types.DiscoveryServers, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() // Not supported with Let's Connect! - if client.isLetsConnect() { - return nil, client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + return nil, errors.Errorf("discovery with Let's Connect is not supported") } - servers, serversErr := client.Discovery.Servers() - if serversErr != nil { - return nil, client.handleError( - errorMessage, - serversErr, - ) - } - return servers, nil + return c.Discovery.Servers() } // GetTranslated gets the translation for `languages` using the current state language. -func (client *Client) GetTranslated(languages map[string]string) string { - return util.GetLanguageMatched(languages, client.Language) -} - -type LetsConnectNotSupportedError struct{} - -func (e LetsConnectNotSupportedError) Error() string { - return "Any operation that involves discovery is not allowed with the Let's Connect! client" +func (c *Client) GetTranslated(languages map[string]string) string { + return util.GetLanguageMatched(languages, c.Language) } diff --git a/client/client_test.go b/client/client_test.go index 23e97ca..57196e4 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1,7 +1,6 @@ package client import ( - "errors" "fmt" "net/http" "os" @@ -12,9 +11,8 @@ import ( "time" httpw "github.com/eduvpn/eduvpn-common/internal/http" - "github.com/eduvpn/eduvpn-common/internal/oauth" "github.com/eduvpn/eduvpn-common/internal/util" - "github.com/eduvpn/eduvpn-common/types" + "github.com/go-errors/errors" ) func getServerURI(t *testing.T) string { @@ -29,7 +27,7 @@ func getServerURI(t *testing.T) string { return serverURI } -func runCommand(t *testing.T, errBuffer *strings.Builder, name string, args ...string) error { +func runCommand(errBuffer *strings.Builder, name string, args ...string) error { cmd := exec.Command(name, args...) cmd.Stderr = errBuffer @@ -41,11 +39,11 @@ func runCommand(t *testing.T, errBuffer *strings.Builder, name string, args ...s return cmd.Wait() } -func loginOAuthSelenium(t *testing.T, url string, state *Client) { +func loginOAuthSelenium(url string, state *Client) { // We could use the go selenium library // But it does not support the latest selenium v4 just yet var errBuffer strings.Builder - err := runCommand(t, &errBuffer, "python3", "../selenium_eduvpn.py", url) + err := runCommand(&errBuffer, "python3", "../selenium_eduvpn.py", url) if err != nil { _ = state.CancelOAuth() panic(fmt.Sprintf( @@ -58,7 +56,7 @@ func loginOAuthSelenium(t *testing.T, url string, state *Client) { func stateCallback( t *testing.T, - oldState FSMStateID, + _ FSMStateID, newState FSMStateID, data interface{}, state *Client, @@ -69,7 +67,7 @@ func stateCallback( if !ok { t.Fatalf("data is not a string for OAuth URL") } - loginOAuthSelenium(t, url, state) + loginOAuthSelenium(url, state) } } @@ -104,7 +102,7 @@ func TestServer(t *testing.T) { func testConnectOAuthParameter( t *testing.T, parameters httpw.URLParameters, - expectedErr interface{}, + errPrefix string, ) { serverURI := getServerURI(t) state := &Client{} @@ -151,55 +149,68 @@ func testConnectOAuthParameter( t.Fatalf("Register error: %v", registerErr) } - _, addErr := state.AddCustomServer(serverURI) + _, err := state.AddCustomServer(serverURI) - var wrappedErr *types.WrappedErrorMessage + if errPrefix == "" { + if err != nil { + t.Fatalf("unexpected error %v", err) + } + return + } + if err == nil { + t.Fatalf("expected error with prefix '%s' but got nil", errPrefix) + } + + err1, ok := err.(*errors.Error) // We ensure the error is of a wrappedErrorMessage - if !errors.As(addErr, &wrappedErr) { - t.Fatalf("error %T = %v, wantErr %T", addErr, addErr, wrappedErr) + if !ok { + t.Fatalf("error %T = %v, wantErr %T", err, err, &errors.Error{}) } - gotExpectedErr := wrappedErr.Cause() + msg := err1.Error() + if err1.Err != nil { + msg = err1.Err.Error() + } // Then we check if the cause is correct - if !errors.As(gotExpectedErr, expectedErr) { - t.Fatalf("error %T = %v, wantErr %T", gotExpectedErr, gotExpectedErr, expectedErr) + if !strings.HasPrefix(msg, errPrefix) { + t.Fatalf("expected error with prefix '%s' but got '%s'", errPrefix, msg) } } func TestConnectOAuthParameters(t *testing.T) { - var ( - failedCallbackParameterError *oauth.CallbackParameterError - failedCallbackStateMatchError *oauth.CallbackStateMatchError - failedCallbackISSMatchError *oauth.CallbackISSMatchError + const ( + callbackParameterErrorPrefix = "failed retrieving parameter '" + callbackStateMatchErrorPrefix = "failed matching state" + callbackISSMatchErrorPrefix = "failed matching ISS" ) serverURI := getServerURI(t) // serverURI already ends with a / due to using the util EnsureValidURL function iss := serverURI tests := []struct { - expectedErr interface{} - parameters httpw.URLParameters + errPrefix string + parameters httpw.URLParameters }{ // missing state and code - {&failedCallbackParameterError, httpw.URLParameters{"iss": iss}}, + {callbackParameterErrorPrefix, httpw.URLParameters{"iss": iss}}, // missing state - {&failedCallbackParameterError, httpw.URLParameters{"iss": iss, "code": "42"}}, + {callbackParameterErrorPrefix, httpw.URLParameters{"iss": iss, "code": "42"}}, // invalid state { - &failedCallbackStateMatchError, + callbackStateMatchErrorPrefix, httpw.URLParameters{"iss": iss, "code": "42", "state": "21"}, }, // invalid iss { - &failedCallbackISSMatchError, + callbackISSMatchErrorPrefix, httpw.URLParameters{"iss": "37", "code": "42", "state": "21"}, }, } for _, test := range tests { - testConnectOAuthParameter(t, test.parameters, test.expectedErr) + testConnectOAuthParameter(t, test.parameters, test.errPrefix) } } diff --git a/client/fsm.go b/client/fsm.go index 316dea4..aa3401b 100644 --- a/client/fsm.go +++ b/client/fsm.go @@ -1,12 +1,9 @@ package client import ( - "errors" - "fmt" - "github.com/eduvpn/eduvpn-common/internal/fsm" "github.com/eduvpn/eduvpn-common/internal/server" - "github.com/eduvpn/eduvpn-common/types" + "github.com/go-errors/errors" ) type ( @@ -206,151 +203,90 @@ func newFSM( return returnedFSM } -type FSMDeregisteredError struct{} - -func (e FSMDeregisteredError) CustomError() *types.WrappedErrorMessage { - return types.NewWrappedError( - "Client not registered with the GO library", - errors.New( - "the current FSM state is deregistered, but the function needs a state that is not deregistered", - ), - ) -} - -type FSMWrongStateTransitionError struct { - Got FSMStateID - Want FSMStateID -} - -func (e FSMWrongStateTransitionError) CustomError() *types.WrappedErrorMessage { - return types.NewWrappedError( - "Wrong FSM transition", - fmt.Errorf( - "wrong FSM state, got: %s, want: a state with a transition to: %s", - GetStateName(e.Got), - GetStateName(e.Want), - ), - ) -} - -type FSMWrongStateError struct { - Got FSMStateID - Want FSMStateID -} - -func (e FSMWrongStateError) CustomError() *types.WrappedErrorMessage { - return types.NewWrappedError( - "Wrong FSM State", - fmt.Errorf( - "wrong FSM state, got: %s, want: %s", - GetStateName(e.Got), - GetStateName(e.Want), - ), - ) -} - // SetSearchServer sets the FSM to the SEARCH_SERVER state. // This indicates that the user wants to search for a new server. // Returns an error if this state transition is not possible. -func (client *Client) SetSearchServer() error { - if !client.FSM.HasTransition(StateSearchServer) { - return client.handleError( - "failed to set search server", - FSMWrongStateTransitionError{ - Got: client.FSM.Current, - Want: StateSearchServer, - }.CustomError(), - ) +func (c *Client) SetSearchServer() error { + if err := c.FSM.CheckTransition(StateSearchServer); err != nil { + c.logError(err) + return err } - client.FSM.GoTransition(StateSearchServer) + //TODO(jwijenbergh): Should we handle `false` returned value here? + c.FSM.GoTransition(StateSearchServer) return nil } // SetConnected sets the FSM to the CONNECTED state. // This indicates that the VPN is connected to the server. // Returns an error if this state transition is not possible. -func (client *Client) SetConnected() error { - errorMessage := "failed to set connected" - if client.InFSMState(StateConnected) { +func (c *Client) SetConnected() error { + if c.InFSMState(StateConnected) { // already connected, show no error - client.Logger.Warningf("Already connected") + c.Logger.Warningf("Already connected") return nil } - if !client.FSM.HasTransition(StateConnected) { - return client.handleError( - errorMessage, - FSMWrongStateTransitionError{ - Got: client.FSM.Current, - Want: StateConnected, - }.CustomError(), - ) + + if err := c.FSM.CheckTransition(StateConnected); err != nil { + c.logError(err) + return err } - currentServer, currentServerErr := client.Servers.GetCurrentServer() - if currentServerErr != nil { - return client.handleError(errorMessage, currentServerErr) + srv, err := c.Servers.GetCurrentServer() + if err != nil { + c.logError(err) + return err } - client.FSM.GoTransitionWithData(StateConnected, currentServer) + c.FSM.GoTransitionWithData(StateConnected, srv) return nil } // SetConnecting sets the FSM to the CONNECTING state. // This indicates that the VPN is currently connecting to the server. // Returns an error if this state transition is not possible. -func (client *Client) SetConnecting() error { - errorMessage := "failed to set connecting" - if client.InFSMState(StateConnecting) { +func (c *Client) SetConnecting() error { + if c.InFSMState(StateConnecting) { // already loading connection, show no error - client.Logger.Warningf("Already connecting") + c.Logger.Warningf("Already connecting") return nil } - if !client.FSM.HasTransition(StateConnecting) { - return client.handleError( - errorMessage, - FSMWrongStateTransitionError{ - Got: client.FSM.Current, - Want: StateConnecting, - }.CustomError(), - ) + if err := c.FSM.CheckTransition(StateConnecting); err != nil { + c.logError(err) + return err } - currentServer, currentServerErr := client.Servers.GetCurrentServer() - if currentServerErr != nil { - return client.handleError(errorMessage, currentServerErr) + srv, err := c.Servers.GetCurrentServer() + if err != nil { + c.logError(err) + return err } - client.FSM.GoTransitionWithData(StateConnecting, currentServer) + c.FSM.GoTransitionWithData(StateConnecting, srv) return nil } // SetDisconnecting sets the FSM to the DISCONNECTING state. // This indicates that the VPN is currently disconnecting from the server. // Returns an error if this state transition is not possible. -func (client *Client) SetDisconnecting() error { - errorMessage := "failed to set disconnecting" - if client.InFSMState(StateDisconnecting) { +func (c *Client) SetDisconnecting() error { + if c.InFSMState(StateDisconnecting) { // already disconnecting, show no error - client.Logger.Warningf("Already disconnecting") + c.Logger.Warningf("Already disconnecting") return nil } - if !client.FSM.HasTransition(StateDisconnecting) { - return client.handleError( - errorMessage, - FSMWrongStateTransitionError{ - Got: client.FSM.Current, - Want: StateDisconnecting, - }.CustomError(), - ) + if err := c.FSM.CheckTransition(StateDisconnecting); err != nil { + c.logError(err) + return err } - currentServer, currentServerErr := client.Servers.GetCurrentServer() - if currentServerErr != nil { - return client.handleError(errorMessage, currentServerErr) + srv, err := c.Servers.GetCurrentServer() + if err != nil { + c.logError(err) + return err } - client.FSM.GoTransitionWithData(StateDisconnecting, currentServer) + c.FSM.GoTransitionWithData(StateDisconnecting, srv) return nil } @@ -358,90 +294,73 @@ func (client *Client) SetDisconnecting() error { // This indicates that the VPN is currently disconnected from the server. // This also sends the /disconnect API call to the server. // Returns an error if this state transition is not possible. -func (client *Client) SetDisconnected(cleanup bool) error { - errorMessage := "failed to set disconnected" - if client.InFSMState(StateDisconnected) { +func (c *Client) SetDisconnected(cleanup bool) error { + if c.InFSMState(StateDisconnected) { // already disconnected, show no error - client.Logger.Warningf("Already disconnected") + c.Logger.Warningf("Already disconnected") return nil } - if !client.FSM.HasTransition(StateDisconnected) { - return client.handleError( - errorMessage, - FSMWrongStateTransitionError{ - Got: client.FSM.Current, - Want: StateDisconnected, - }.CustomError(), - ) + if err := c.FSM.CheckTransition(StateDisconnected); err != nil { + c.logError(err) + return err } - currentServer, currentServerErr := client.Servers.GetCurrentServer() - if currentServerErr != nil { - return client.handleError(errorMessage, currentServerErr) + srv, err := c.Servers.GetCurrentServer() + if err != nil { + c.logError(err) + return err } if cleanup { // Do the /disconnect API call and go to disconnected after... - server.Disconnect(currentServer) + server.Disconnect(srv) } - client.FSM.GoTransitionWithData(StateDisconnected, currentServer) + c.FSM.GoTransitionWithData(StateDisconnected, srv) return nil } // goBackInternal uses the public go back but logs an error if it happened. -func (client *Client) goBackInternal() { - goBackErr := client.GoBack() - if goBackErr != nil { - client.Logger.Infof( - fmt.Sprintf( - "Failed going back, error: %s", - types.ErrorTraceback(goBackErr), - ), - ) +func (c *Client) goBackInternal() { + err := c.GoBack() + if err != nil { + //TODO(jwijenbergh): Bit suspicious - logging level INFO, yet stacktrace logged. + c.Logger.Infof("failed going back: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } } // GoBack transitions the FSM back to the previous UI state, for now this is always the NO_SERVER state. -func (client *Client) GoBack() error { - errorMessage := "failed to go back" - if client.InFSMState(StateDeregistered) { - return client.handleError( - errorMessage, - FSMDeregisteredError{}.CustomError(), - ) +func (c *Client) GoBack() error { + if c.InFSMState(StateDeregistered) { + err := errors.Errorf("fms attempt going back from 'StateDeregistered'") + c.logError(err) + return err } - // FIXME: Abitrary back transitions don't work because we need the approriate data - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) + // FIXME: Arbitrary back transitions don't work because we need the appropriate data + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) return nil } // CancelOAuth cancels OAuth if one is in progress. // If OAuth is not in progress, it returns an error. -// An error is also returned if OAuth is in progress but it fails to cancel it. -func (client *Client) CancelOAuth() error { - errorMessage := "failed to cancel OAuth" - if !client.InFSMState(StateOAuthStarted) { - return client.handleError( - errorMessage, - FSMWrongStateError{ - Got: client.FSM.Current, - Want: StateOAuthStarted, - }.CustomError(), - ) +// An error is also returned if OAuth is in progress, but it fails to cancel it. +func (c *Client) CancelOAuth() error { + if !c.InFSMState(StateOAuthStarted) { + return errors.Errorf("fsm attempt cancelling OAuth while in '%v'", c.FSM.Current) } - currentServer, serverErr := client.Servers.GetCurrentServer() - if serverErr != nil { - return client.handleError(errorMessage, serverErr) + srv, err := c.Servers.GetCurrentServer() + if err != nil { + c.logError(err) + } else { + server.CancelOAuth(srv) } - server.CancelOAuth(currentServer) - return nil + return err } // InFSMState is a helper to check if the FSM is in state `checkState`. -func (client *Client) InFSMState(checkState FSMStateID) bool { - return client.FSM.InState(checkState) +func (c *Client) InFSMState(checkState FSMStateID) bool { + return c.FSM.InState(checkState) } diff --git a/client/server.go b/client/server.go index f3a161b..333687f 100644 --- a/client/server.go +++ b/client/server.go @@ -1,160 +1,139 @@ package client import ( - "errors" - "fmt" - "github.com/eduvpn/eduvpn-common/internal/oauth" "github.com/eduvpn/eduvpn-common/internal/server" "github.com/eduvpn/eduvpn-common/internal/util" "github.com/eduvpn/eduvpn-common/types" + "github.com/go-errors/errors" ) // getConfigAuth gets a config with authorization and authentication. // It also asks for a profile if no valid profile is found. -func (client *Client) getConfigAuth( - chosenServer server.Server, - preferTCP bool, -) (string, string, error) { - loginErr := client.ensureLogin(chosenServer) - if loginErr != nil { - return "", "", loginErr +func (c *Client) getConfigAuth(srv server.Server, preferTCP bool) (string, string, error) { + err := c.ensureLogin(srv) + if err != nil { + return "", "", err } - client.FSM.GoTransition(StateRequestConfig) - validProfile, profileErr := server.HasValidProfile(chosenServer, client.SupportsWireguard) - if profileErr != nil { - return "", "", profileErr + //TODO(jwijenbergh): Should we check if it returns false? + c.FSM.GoTransition(StateRequestConfig) + + ok, err := server.HasValidProfile(srv, c.SupportsWireguard) + if err != nil { + return "", "", err } // No valid profile, ask for one - if !validProfile { - askProfileErr := client.askProfile(chosenServer) - if askProfileErr != nil { - return "", "", askProfileErr + if !ok { + if err = c.askProfile(srv); err != nil { + return "", "", err } } // We return the error otherwise we wrap it too much - return server.Config(chosenServer, client.SupportsWireguard, preferTCP) + return server.Config(srv, c.SupportsWireguard, preferTCP) } // retryConfigAuth retries the getConfigAuth function if the tokens are invalid. // If OAuth is cancelled, it makes sure that we only forward the error as additional info. -func (client *Client) retryConfigAuth( - chosenServer server.Server, - preferTCP bool, -) (string, string, error) { - errorMessage := "failed authorized config retry" - config, configType, configErr := client.getConfigAuth(chosenServer, preferTCP) - if configErr != nil { - var error *oauth.TokensInvalidError - +func (c *Client) retryConfigAuth(srv server.Server, preferTCP bool) (string, string, error) { + cfg, cfgType, err := c.getConfigAuth(srv, preferTCP) + if err == nil { + return cfg, cfgType, nil + } + if err != nil { // Only retry if the error is that the tokens are invalid - if errors.As(configErr, &error) { - config, configType, configErr = client.getConfigAuth( - chosenServer, - preferTCP, - ) - if configErr == nil { - return config, configType, nil + if errors.As(err, &oauth.TokensInvalidError{}) { + cfg, cfgType, err = c.getConfigAuth(srv, preferTCP) + if err == nil { + return cfg, cfgType, nil } } - client.goBackInternal() - return "", "", types.NewWrappedError(errorMessage, configErr) + c.goBackInternal() } - return config, configType, nil + return "", "", err } // getConfig gets an OpenVPN/WireGuard configuration by contacting the server, moving the FSM towards the DISCONNECTED state and then saving the local configuration file. -func (client *Client) getConfig( - chosenServer server.Server, - preferTCP bool, -) (string, string, error) { - errorMessage := "failed to get a configuration for OpenVPN/Wireguard" - if client.InFSMState(StateDeregistered) { - return "", "", types.NewWrappedError( - errorMessage, - FSMDeregisteredError{}.CustomError(), - ) +func (c *Client) getConfig(srv server.Server, preferTCP bool) (string, string, error) { + if c.InFSMState(StateDeregistered) { + return "", "", errors.Errorf("getConfig attempt in '%v'", StateDeregistered) } // Refresh the server endpoints - // This is best effort - endpointErr := server.RefreshEndpoints(chosenServer) - if endpointErr != nil { - client.Logger.Warningf("failed to refresh server endpoints: %v", endpointErr) + // This is the best effort + err := server.RefreshEndpoints(srv) + if err != nil { + c.Logger.Warningf("failed to refresh server endpoints: %v", err) } - config, configType, configErr := client.retryConfigAuth(chosenServer, preferTCP) - if configErr != nil { - return "", "", types.NewWrappedError(errorMessage, configErr) + cfg, cfgType, err := c.retryConfigAuth(srv, preferTCP) + if err != nil { + return "", "", err } - currentServer, currentServerErr := client.Servers.GetCurrentServer() - if currentServerErr != nil { - return "", "", types.NewWrappedError(errorMessage, currentServerErr) + srv1, err := c.Servers.GetCurrentServer() + if err != nil { + return "", "", err } // Signal the server display info - client.FSM.GoTransitionWithData(StateDisconnected, currentServer) + c.FSM.GoTransitionWithData(StateDisconnected, srv1) // Save the config - saveErr := client.Config.Save(&client) - if saveErr != nil { - client.Logger.Infof( - "Failed saving configuration after getting a server: %s", - types.ErrorTraceback(saveErr), - ) + if err = c.Config.Save(&c); err != nil { + //TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... + //TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. + c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + err.Error(), err.(*errors.Error).ErrorStack()) } - return config, configType, nil + return cfg, cfgType, nil } // SetSecureLocation sets the location for the current secure location server. countryCode is the secure location to be chosen. // This function returns an error e.g. if the server cannot be found or the location is wrong. -func (client *Client) SetSecureLocation(countryCode string) error { - errorMessage := "failed asking secure location" - +func (c *Client) SetSecureLocation(countryCode string) error { // Not supported with Let's Connect! - if client.isLetsConnect() { - return client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + err := errors.Errorf("discovery with Let's Connect is not supported") + c.logError(err) + return err } - server, serverErr := client.Discovery.ServerByCountryCode(countryCode, "secure_internet") - if serverErr != nil { - client.goBackInternal() - return client.handleError(errorMessage, serverErr) + srv, err := c.Discovery.ServerByCountryCode(countryCode, "secure_internet") + if err != nil { + c.goBackInternal() + c.logError(err) + return err } - setLocationErr := client.Servers.SetSecureLocation(server) - if setLocationErr != nil { - client.goBackInternal() - return client.handleError(errorMessage, setLocationErr) + if err = c.Servers.SetSecureLocation(srv); err != nil { + c.goBackInternal() + c.logError(err) } - return nil + return err } // RemoveSecureInternet removes the current secure internet server. // It returns an error if the server cannot be removed due to the state being DEREGISTERED. // Note that if the server does not exist, it returns nil as an error. -func (client *Client) RemoveSecureInternet() error { - if client.InFSMState(StateDeregistered) { - return client.handleError( - "failed to remove Secure Internet", - FSMDeregisteredError{}.CustomError(), - ) +func (c *Client) RemoveSecureInternet() error { + if c.InFSMState(StateDeregistered) { + err := errors.Errorf("RemoveSecureInternet attempt in '%v'", StateDeregistered) + c.logError(err) + return err } // No error because we can only have one secure internet server and if there are no secure internet servers, this is a NO-OP - client.Servers.RemoveSecureInternet() - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) + c.Servers.RemoveSecureInternet() + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) // Save the config - saveErr := client.Config.Save(&client) - if saveErr != nil { - client.Logger.Infof( - "Failed saving configuration after removing a secure internet server: %s", - types.ErrorTraceback(saveErr), - ) + if err := c.Config.Save(&c); err != nil { + //TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... + //TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. + c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + err.Error(), err.(*errors.Error).ErrorStack()) } return nil } @@ -162,23 +141,21 @@ func (client *Client) RemoveSecureInternet() error { // RemoveInstituteAccess removes the institute access server with `url`. // It returns an error if the server cannot be removed due to the state being DEREGISTERED. // Note that if the server does not exist, it returns nil as an error. -func (client *Client) RemoveInstituteAccess(url string) error { - if client.InFSMState(StateDeregistered) { - return client.handleError( - "failed to remove Institute Access", - FSMDeregisteredError{}.CustomError(), - ) +func (c *Client) RemoveInstituteAccess(url string) error { + if c.InFSMState(StateDeregistered) { + err := errors.Errorf("RemoveInstituteAccess attempt in '%v'", StateDeregistered) + c.logError(err) + return err } // No error because this is a NO-OP if the server doesn't exist - client.Servers.RemoveInstituteAccess(url) - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) + c.Servers.RemoveInstituteAccess(url) + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) // Save the config - saveErr := client.Config.Save(&client) - if saveErr != nil { - client.Logger.Infof( - "Failed saving configuration after removing an institute access server: %s", - types.ErrorTraceback(saveErr), - ) + if err := c.Config.Save(&c); err != nil { + //TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... + //TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. + c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + err.Error(), err.(*errors.Error).ErrorStack()) } return nil } @@ -186,146 +163,149 @@ func (client *Client) RemoveInstituteAccess(url string) error { // RemoveCustomServer removes the custom server with `url`. // It returns an error if the server cannot be removed due to the state being DEREGISTERED. // Note that if the server does not exist, it returns nil as an error. -func (client *Client) RemoveCustomServer(url string) error { - if client.InFSMState(StateDeregistered) { - return client.handleError( - "failed to remove Custom Server", - FSMDeregisteredError{}.CustomError(), - ) +func (c *Client) RemoveCustomServer(url string) error { + if c.InFSMState(StateDeregistered) { + err := errors.Errorf("RemoveCustomServer attempt in '%v'", StateDeregistered) + c.logError(err) + return err } // No error because this is a NO-OP if the server doesn't exist - client.Servers.RemoveCustomServer(url) - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) + c.Servers.RemoveCustomServer(url) + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) // Save the config - saveErr := client.Config.Save(&client) - if saveErr != nil { - client.Logger.Infof( - "Failed saving configuration after removing a custom server: %s", - types.ErrorTraceback(saveErr), - ) + if err := c.Config.Save(&c); err != nil { + //TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... + //TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. + c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + err.Error(), err.(*errors.Error).ErrorStack()) } return nil } // AddInstituteServer adds an Institute Access server by `url`. -func (client *Client) AddInstituteServer(url string) (server.Server, error) { - errorMessage := fmt.Sprintf("failed adding Institute Access server with url %s", url) +func (c *Client) AddInstituteServer(url string) (srv server.Server, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() // Not supported with Let's Connect! - if client.isLetsConnect() { - return nil, client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + err = errors.Errorf("discovery with Let's Connect is not supported") + return nil, err } // Indicate that we're loading the server - client.FSM.GoTransition(StateLoadingServer) + c.FSM.GoTransition(StateLoadingServer) // FIXME: Do nothing with discovery here as the client already has it // So pass a server as the parameter - instituteServer, discoErr := client.Discovery.ServerByURL(url, "institute_access") - if discoErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, discoErr) + var dSrv *types.DiscoveryServer + dSrv, err = c.Discovery.ServerByURL(url, "institute_access") + if err != nil { + c.goBackInternal() + return nil, err } // Add the secure internet server - server, serverErr := client.Servers.AddInstituteAccessServer(instituteServer) - if serverErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, serverErr) + srv, err = c.Servers.AddInstituteAccessServer(dSrv) + if err != nil { + c.goBackInternal() + return nil, err } // Set the server as the current so OAuth can be cancelled - currentErr := client.Servers.SetInstituteAccess(server) - if currentErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, currentErr) + if err = c.Servers.SetInstituteAccess(srv); err != nil { + c.goBackInternal() + return nil, err } // Indicate that we want to authorize this server - client.FSM.GoTransition(StateChosenServer) + c.FSM.GoTransition(StateChosenServer) // Authorize it - loginErr := client.ensureLogin(server) - if loginErr != nil { + if err = c.ensureLogin(srv); err != nil { // Removing is best effort - _ = client.RemoveInstituteAccess(url) - return nil, client.handleError(errorMessage, loginErr) + _ = c.RemoveInstituteAccess(url) + return nil, err } - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) - return server, nil + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) + return srv, nil } // AddSecureInternetHomeServer adds a Secure Internet Home Server with `orgID` that was obtained from the Discovery file. // Because there is only one Secure Internet Home Server, it replaces the existing one. -func (client *Client) AddSecureInternetHomeServer(orgID string) (server.Server, error) { - errorMessage := fmt.Sprintf( - "failed adding Secure Internet home server with organization ID %s", - orgID, - ) +func (c *Client) AddSecureInternetHomeServer(orgID string) (srv server.Server, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() // Not supported with Let's Connect! - if client.isLetsConnect() { - return nil, client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + return nil, errors.Errorf("discovery with Let's Connect is not supported") } // Indicate that we're loading the server - client.FSM.GoTransition(StateLoadingServer) + c.FSM.GoTransition(StateLoadingServer) // Get the secure internet URL from discovery - secureOrg, secureServer, discoErr := client.Discovery.SecureHomeArgs(orgID) - if discoErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, discoErr) + org, dSrv, err := c.Discovery.SecureHomeArgs(orgID) + if err != nil { + c.goBackInternal() + return nil, err } // Add the secure internet server - server, serverErr := client.Servers.AddSecureInternet(secureOrg, secureServer) - if serverErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, serverErr) + srv, err = c.Servers.AddSecureInternet(org, dSrv) + if err != nil { + return nil, err } - locationErr := client.askSecureLocation() - if locationErr != nil { - // Removing is best effort + //TODO(jwijenbergh): Does this call transfers execution flow to UI? + if err = c.askSecureLocation(); err != nil { + // Removing is the best effort // This already goes back to the main screen - _ = client.RemoveSecureInternet() - return nil, client.handleError(errorMessage, locationErr) + _ = c.RemoveSecureInternet() + return nil, err } // Set the server as the current so OAuth can be cancelled - currentErr := client.Servers.SetSecureInternet(server) - if currentErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, currentErr) + if err = c.Servers.SetSecureInternet(srv); err != nil { + c.goBackInternal() + return nil, err } // Server has been chosen for authentication - client.FSM.GoTransition(StateChosenServer) + c.FSM.GoTransition(StateChosenServer) // Authorize it - loginErr := client.ensureLogin(server) - if loginErr != nil { + if err = c.ensureLogin(srv); err != nil { // Removing is best effort - _ = client.RemoveSecureInternet() - return nil, client.handleError(errorMessage, loginErr) + _ = c.RemoveSecureInternet() + return nil, err } - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) - return server, nil + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) + return srv, nil } // AddCustomServer adds a Custom Server by `url`. -func (client *Client) AddCustomServer(url string) (server.Server, error) { - errorMessage := fmt.Sprintf("failed adding Custom server with url %s", url) +func (c *Client) AddCustomServer(url string) (srv server.Server, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() - url, urlErr := util.EnsureValidURL(url) - if urlErr != nil { - return nil, client.handleError(errorMessage, urlErr) + if url, err = util.EnsureValidURL(url); err != nil { + return nil, err } // Indicate that we're loading the server - client.FSM.GoTransition(StateLoadingServer) + c.FSM.GoTransition(StateLoadingServer) customServer := &types.DiscoveryServer{ BaseURL: url, @@ -334,167 +314,160 @@ func (client *Client) AddCustomServer(url string) (server.Server, error) { } // A custom server is just an institute access server under the hood - server, serverErr := client.Servers.AddCustomServer(customServer) - if serverErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, serverErr) + if srv, err = c.Servers.AddCustomServer(customServer); err != nil { + c.goBackInternal() + return nil, err } // Set the server as the current so OAuth can be cancelled - currentErr := client.Servers.SetCustomServer(server) - if currentErr != nil { - client.goBackInternal() - return nil, client.handleError(errorMessage, currentErr) + if err = c.Servers.SetCustomServer(srv); err != nil { + c.goBackInternal() + return nil, err } // Server has been chosen for authentication - client.FSM.GoTransition(StateChosenServer) + c.FSM.GoTransition(StateChosenServer) // Authorize it - loginErr := client.ensureLogin(server) - if loginErr != nil { + if err = c.ensureLogin(srv); err != nil { // removing is best effort - _ = client.RemoveCustomServer(url) - return nil, client.handleError(errorMessage, loginErr) + _ = c.RemoveCustomServer(url) + return nil, err } - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) - return server, nil + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) + return srv, nil } // GetConfigInstituteAccess gets a configuration for an Institute Access Server. // It ensures that the Institute Access Server exists by creating or using an existing one with the url. // `preferTCP` indicates that the client wants to use TCP (through OpenVPN) to establish the VPN tunnel. -func (client *Client) GetConfigInstituteAccess(url string, preferTCP bool) (string, string, error) { - errorMessage := fmt.Sprintf("failed getting a configuration for Institute Access %s", url) +func (c *Client) GetConfigInstituteAccess(url string, preferTCP bool) (cfg string, cfgType string, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() // Not supported with Let's Connect! - if client.isLetsConnect() { - return "", "", client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + return "", "", errors.Errorf("discovery with Let's Connect is not supported") } - client.FSM.GoTransition(StateLoadingServer) + c.FSM.GoTransition(StateLoadingServer) // Get the server if it exists - server, serverErr := client.Servers.GetInstituteAccess(url) - if serverErr != nil { - client.goBackInternal() - return "", "", client.handleError(errorMessage, serverErr) + var iaSrv *server.InstituteAccessServer + if iaSrv, err = c.Servers.GetInstituteAccess(url); err != nil { + c.goBackInternal() + return "", "", err } // Set the server as the current - currentErr := client.Servers.SetInstituteAccess(server) - if currentErr != nil { - return "", "", client.handleError(errorMessage, currentErr) + if err = c.Servers.SetInstituteAccess(iaSrv); err != nil { + return "", "", err } // The server has now been chosen - client.FSM.GoTransition(StateChosenServer) + c.FSM.GoTransition(StateChosenServer) - config, configType, configErr := client.getConfig(server, preferTCP) - if configErr != nil { - client.goBackInternal() - return "", "", client.handleError(errorMessage, configErr) + if cfg, cfgType, err = c.getConfig(iaSrv, preferTCP); err != nil { + c.goBackInternal() } - return config, configType, nil + + return cfg, cfgType, err } // GetConfigSecureInternet gets a configuration for a Secure Internet Server. // It ensures that the Secure Internet Server exists by creating or using an existing one with the orgID. // `preferTCP` indicates that the client wants to use TCP (through OpenVPN) to establish the VPN tunnel. -func (client *Client) GetConfigSecureInternet( - orgID string, - preferTCP bool, -) (string, string, error) { - errorMessage := fmt.Sprintf( - "failed getting a configuration for Secure Internet organization %s", - orgID, - ) +func (c *Client) GetConfigSecureInternet(orgID string, preferTCP bool) (cfg string, cfgType string, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() // Not supported with Let's Connect! - if client.isLetsConnect() { - return "", "", client.handleError(errorMessage, LetsConnectNotSupportedError{}) + if c.isLetsConnect() { + return "", "", errors.Errorf("discovery with Let's Connect is not supported") } - client.FSM.GoTransition(StateLoadingServer) + c.FSM.GoTransition(StateLoadingServer) // Get the server if it exists - server, serverErr := client.Servers.GetSecureInternetHomeServer() - if serverErr != nil { - client.goBackInternal() - return "", "", client.handleError(errorMessage, serverErr) + var srv *server.SecureInternetHomeServer + if srv, err = c.Servers.GetSecureInternetHomeServer(); err != nil { + c.goBackInternal() + return "", "", err } // Set the server as the current - currentErr := client.Servers.SetSecureInternet(server) - if currentErr != nil { - return "", "", client.handleError(errorMessage, currentErr) + if err = c.Servers.SetSecureInternet(srv); err != nil { + return "", "", err } - client.FSM.GoTransition(StateChosenServer) + c.FSM.GoTransition(StateChosenServer) - config, configType, configErr := client.getConfig(server, preferTCP) - if configErr != nil { - client.goBackInternal() - return "", "", client.handleError(errorMessage, configErr) + if cfg, cfgType, err = c.getConfig(srv, preferTCP); err != nil { + c.goBackInternal() } - return config, configType, nil + + return cfg, cfgType, err } // GetConfigCustomServer gets a configuration for a Custom Server. // It ensures that the Custom Server exists by creating or using an existing one with the url. // `preferTCP` indicates that the client wants to use TCP (through OpenVPN) to establish the VPN tunnel. -func (client *Client) GetConfigCustomServer(url string, preferTCP bool) (string, string, error) { - errorMessage := fmt.Sprintf("failed getting a configuration for custom server %s", url) +func (c *Client) GetConfigCustomServer(url string, preferTCP bool) (cfg string, cfgType string, err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() - url, urlErr := util.EnsureValidURL(url) - if urlErr != nil { - return "", "", client.handleError(errorMessage, urlErr) + if url, err = util.EnsureValidURL(url); err != nil { + return "", "", err } - client.FSM.GoTransition(StateLoadingServer) + c.FSM.GoTransition(StateLoadingServer) // Get the server if it exists - server, serverErr := client.Servers.GetCustomServer(url) - if serverErr != nil { - client.goBackInternal() - return "", "", client.handleError(errorMessage, serverErr) + var srv *server.InstituteAccessServer + if srv, err = c.Servers.GetCustomServer(url); err != nil { + c.goBackInternal() + return "", "", err } // Set the server as the current - currentErr := client.Servers.SetCustomServer(server) - if currentErr != nil { - return "", "", client.handleError(errorMessage, currentErr) + if err = c.Servers.SetCustomServer(srv); err != nil { + return "", "", err } - client.FSM.GoTransition(StateChosenServer) + c.FSM.GoTransition(StateChosenServer) - config, configType, configErr := client.getConfig(server, preferTCP) - if configErr != nil { - client.goBackInternal() - return "", "", client.handleError(errorMessage, configErr) + if cfg, cfgType, err = c.getConfig(srv, preferTCP); err != nil { + c.goBackInternal() } - return config, configType, nil + + return cfg, cfgType, err } // askSecureLocation asks the user to choose a Secure Internet location by moving the FSM to the STATE_ASK_LOCATION state. -func (client *Client) askSecureLocation() error { - errorMessage := "failed settings secure location" - locations := client.Discovery.SecureLocationList() +func (c *Client) askSecureLocation() error { + loc := c.Discovery.SecureLocationList() // Ask for the location in the callback - goTransitionErr := client.FSM.GoTransitionRequired(StateAskLocation, locations) - if goTransitionErr != nil { - return types.NewWrappedError(errorMessage, goTransitionErr) + if err := c.FSM.GoTransitionRequired(StateAskLocation, loc); err != nil { + return err } // The state has changed, meaning setting the secure location was not successful - if client.FSM.Current != StateAskLocation { + if c.FSM.Current != StateAskLocation { // TODO: maybe a custom type for this errors.new? - return types.NewWrappedError( - errorMessage, - errors.New("failed loading secure location"), - ) + // ^^^^ Definitely no! New error types should be introduced only when actually needed. + return errors.Errorf("fsm failed to transit; expected %v / actual %v", + StateAskLocation, c.FSM.Current) } return nil } @@ -502,123 +475,113 @@ func (client *Client) askSecureLocation() error { // ChangeSecureLocation changes the location for an existing Secure Internet Server. // Changing a secure internet location is only possible when the user is in the main screen (STATE_NO_SERVER), otherwise it returns an error. // It also returns an error if something has gone wrong when selecting the new location. -func (client *Client) ChangeSecureLocation() error { - errorMessage := "failed to change location from the main screen" - - if !client.InFSMState(StateNoServer) { - return client.handleError( - errorMessage, - FSMWrongStateError{ - Got: client.FSM.Current, - Want: StateNoServer, - }.CustomError(), - ) +func (c *Client) ChangeSecureLocation() error { + if !c.InFSMState(StateNoServer) { + return errors.Errorf("ChangeSecureLocation attempt in %v (only %v allowed)", + c.FSM.Current, StateNoServer) } - askLocationErr := client.askSecureLocation() - if askLocationErr != nil { - return client.handleError(errorMessage, askLocationErr) + if err := c.askSecureLocation(); err != nil { + c.logError(err) + return err } // Go back to the main screen - client.FSM.GoTransitionWithData(StateNoServer, client.Servers) + c.FSM.GoTransitionWithData(StateNoServer, c.Servers) return nil } // RenewSession renews the session for the current VPN server. // This logs the user back in. -func (client *Client) RenewSession() error { - errorMessage := "failed to renew session" +func (c *Client) RenewSession() (err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() - currentServer, currentServerErr := client.Servers.GetCurrentServer() - if currentServerErr != nil { - return client.handleError(errorMessage, currentServerErr) + var srv server.Server + if srv, err = c.Servers.GetCurrentServer(); err != nil { + return err } // The server has not been chosen yet, this means that we want to manually renew - if client.FSM.InState(StateNoServer) { - client.FSM.GoTransition(StateChosenServer) - } - - server.MarkTokensForRenew(currentServer) - loginErr := client.ensureLogin(currentServer) - if loginErr != nil { - return client.handleError(errorMessage, loginErr) + if c.FSM.InState(StateNoServer) { + c.FSM.GoTransition(StateChosenServer) } - return nil + server.MarkTokensForRenew(srv) + return c.ensureLogin(srv) } // ShouldRenewButton returns true if the renew button should be shown // If there is no server then this returns false and logs with INFO if so // In other cases it simply checks the expiry time and calculates according to: https://github.com/eduvpn/documentation/blob/b93854dcdd22050d5f23e401619e0165cb8bc591/API.md#session-expiry. -func (client *Client) ShouldRenewButton() bool { - if !client.InFSMState(StateConnected) && !client.InFSMState(StateConnecting) && - !client.InFSMState(StateDisconnected) && - !client.InFSMState(StateDisconnecting) { +func (c *Client) ShouldRenewButton() bool { + if !c.InFSMState(StateConnected) && !c.InFSMState(StateConnecting) && + !c.InFSMState(StateDisconnected) && + !c.InFSMState(StateDisconnecting) { return false } - currentServer, currentServerErr := client.Servers.GetCurrentServer() - - if currentServerErr != nil { - client.Logger.Infof( - "No server found to renew with err: %s", - types.ErrorTraceback(currentServerErr), - ) + srv, err := c.Servers.GetCurrentServer() + if err != nil { + c.Logger.Infof("no server to renew: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) return false } - return server.ShouldRenewButton(currentServer) + return server.ShouldRenewButton(srv) } // ensureLogin logs the user back in if needed. // It runs the FSM transitions to ask for user input. -func (client *Client) ensureLogin(chosenServer server.Server) error { - errorMessage := "failed ensuring login" +func (c *Client) ensureLogin(srv server.Server) error { // Relogin with oauth // This moves the state to authorized - if server.NeedsRelogin(chosenServer) { - url, urlErr := server.OAuthURL(chosenServer, client.Name) - - goTransitionErr := client.FSM.GoTransitionRequired(StateOAuthStarted, url) - if goTransitionErr != nil { - return types.NewWrappedError(errorMessage, goTransitionErr) - } + if !server.NeedsRelogin(srv) { + // OAuth was valid, ensure we are in the authorized state + c.FSM.GoTransition(StateAuthorized) + return nil + } - if urlErr != nil { - client.goBackInternal() - return types.NewWrappedError(errorMessage, urlErr) - } + url, err := server.OAuthURL(srv, c.Name) + //TODO(jwijenbergh): Check if this if block is needed. + if err != nil { + return nil + } - exchangeErr := server.OAuthExchange(chosenServer) + if err = c.FSM.GoTransitionRequired(StateOAuthStarted, url); err != nil { + return err + } - if exchangeErr != nil { - client.goBackInternal() - return types.NewWrappedError(errorMessage, exchangeErr) - } + if err = server.OAuthExchange(srv); err != nil { + c.goBackInternal() } - // OAuth was valid, ensure we are in the authorized state - client.FSM.GoTransition(StateAuthorized) - return nil + + return err } // SetProfileID sets a `profileID` for the current server. // An error is returned if this is not possible, for example when no server is configured. -func (client *Client) SetProfileID(profileID string) error { - errorMessage := "failed to set the profile ID for the current server" - server, serverErr := client.Servers.GetCurrentServer() - if serverErr != nil { - client.goBackInternal() - return client.handleError(errorMessage, serverErr) +func (c *Client) SetProfileID(profileID string) (err error) { + defer func() { + if err != nil { + c.logError(err) + } + }() + + var srv server.Server + if srv, err = c.Servers.GetCurrentServer(); err != nil { + c.goBackInternal() + return err } - base, baseErr := server.Base() - if baseErr != nil { - client.goBackInternal() - return client.handleError(errorMessage, baseErr) + var b *server.Base + if b, err = srv.Base(); err != nil { + c.goBackInternal() + return err } - base.Profiles.Current = profileID + b.Profiles.Current = profileID return nil } |
