Merge pull request #222 from MysteriumNetwork/fix/MYST-498-cancel-con…

…nection-returns-499

MYST-498 Cancel connection returns 499

client/connection/manager.go

@@ -20,8 +20,6 @@ var (
 	ErrNoConnection = errors.New("no connection exists")
 	// ErrAlreadyExists error indicates that aciton applieto to manager expects no active connection (i.e. connect)
 	ErrAlreadyExists = errors.New("connection already exists")
-	// ErrConnectionCancelled indicates that connection in progress was cancelled by request of api user
-	ErrConnectionCancelled = errors.New("connection was cancelled")
 	// ErrOpenvpnProcessDied indicates that Connect method didn't reach "Connected" phase due to openvpn error
 	ErrOpenvpnProcessDied = errors.New("openvpn process died")
 )
@@ -62,9 +60,13 @@ func (manager *connectionManager) Connect(consumerID, providerID identity.Identi
 		}
 	}()
 
+	return manager.startConnection(consumerID, providerID)
+}
+
+func (manager *connectionManager) startConnection(consumerID, providerID identity.Identity) (err error) {
 	cancelable := utils.NewCancelable()
 	manager.cleanConnection = utils.CallOnce(func() {
-		log.Info(managerLogPrefix, "Canceling connection initiation")
+		log.Info(managerLogPrefix, "Cancelling connection initiation")
 		manager.status = statusDisconnecting()
 		cancelable.Cancel()
 	})
@@ -214,7 +216,7 @@ func (manager *connectionManager) waitForConnectedState(stateChannel <-chan open
 				manager.onStateChanged(state, sessionID)
 			}
 		case <-cancelRequest:
-			return ErrConnectionCancelled
+			return utils.ErrRequestCancelled
 		}
 	}
 }

client/connection/manager_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/mysterium/node/server"
 	"github.com/mysterium/node/service_discovery/dto"
 	"github.com/mysterium/node/session"
+	"github.com/mysterium/node/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 	"sync"
@@ -208,7 +209,7 @@ func (tc *testContext) TestConnectingInProgressCanBeCanceled() {
 
 	connectWaiter.Wait()
 
-	assert.Equal(tc.T(), ErrConnectionCancelled, err)
+	assert.Equal(tc.T(), utils.ErrRequestCancelled, err)
 }
 
 func (tc *testContext) TestConnectMethodReturnsErrorIfOpenvpnClientExitsDuringConnect() {

tequilapi/endpoints/connection.go

@@ -10,6 +10,7 @@ import (
 	"github.com/mysterium/node/openvpn/middlewares/client/bytescount"
 	"github.com/mysterium/node/tequilapi/utils"
 	"github.com/mysterium/node/tequilapi/validation"
+	node_utils "github.com/mysterium/node/utils"
 	"net/http"
 )
 
@@ -71,8 +72,8 @@ func (ce *ConnectionEndpoint) Create(resp http.ResponseWriter, req *http.Request
 		switch err {
 		case connection.ErrAlreadyExists:
 			utils.SendError(resp, err, http.StatusConflict)
-		case connection.ErrConnectionCancelled:
-			utils.SendError(resp, err, statusConnectCancelled)
+		case node_utils.ErrRequestCancelled:
+			utils.SendErrorMessage(resp, "connection was cancelled", statusConnectCancelled)
 		default:
 			log.Error(connectionLogPrefix, err)
 			utils.SendError(resp, err, http.StatusInternalServerError)

tequilapi/endpoints/connection_test.go

@@ -37,7 +37,7 @@ func (fm *fakeManager) Status() connection.ConnectionStatus {
 }
 
 func (fm *fakeManager) Disconnect() error {
-	fm.disconnectCount += 1
+	fm.disconnectCount++
 	return fm.onDisconnectReturn
 }
 
@@ -414,7 +414,7 @@ func TestDisconnectReturnsConflictStatusIfConnectionDoesNotExist(t *testing.T) {
 
 func TestConnectReturnsConnectCancelledStatusWhenErrConnectionCancelledIsEncountered(t *testing.T) {
 	manager := fakeManager{}
-	manager.onConnectReturn = connection.ErrConnectionCancelled
+	manager.onConnectReturn = utils.ErrRequestCancelled
 
 	connectionEndpoint := NewConnectionEndpoint(&manager, nil, nil)
 	req := httptest.NewRequest(

tequilapi/endpoints/health_check_test.go

@@ -56,6 +56,6 @@ func newMockTimer(values []time.Time) *mockTimer {
 
 func (mockTimer *mockTimer) Now() time.Time {
 	value := mockTimer.values[mockTimer.current%len(mockTimer.values)]
-	mockTimer.current += 1
+	mockTimer.current++
 	return value
 }

tequilapi/utils/utils.go

@@ -28,11 +28,16 @@ type errorMessage struct {
 
 // SendError generates error response for error
 func SendError(writer http.ResponseWriter, err error, httpCode int) {
-	SendErrorMessage(writer, &errorMessage{fmt.Sprint(err)}, httpCode)
+	SendErrorMessage(writer, fmt.Sprint(err), httpCode)
 }
 
-// SendErrorMessage generates error response with custom message
-func SendErrorMessage(writer http.ResponseWriter, message interface{}, httpCode int) {
+// SendErrorMessage generates error response with custom json message
+func SendErrorMessage(writer http.ResponseWriter, message string, httpCode int) {
+	SendErrorBody(writer, &errorMessage{message}, httpCode)
+}
+
+// SendErrorBody generates error response with custom body
+func SendErrorBody(writer http.ResponseWriter, message interface{}, httpCode int) {
 	writer.WriteHeader(httpCode)
 	WriteAsJSON(message, writer)
 }
@@ -46,5 +51,5 @@ type validationErrorMessage struct {
 func SendValidationErrorMessage(resp http.ResponseWriter, errorMap *validation.FieldErrorMap) {
 	errorResponse := errorMessage{Message: "validation_error"}
 
-	SendErrorMessage(resp, &validationErrorMessage{errorResponse, errorMap}, http.StatusUnprocessableEntity)
+	SendErrorBody(resp, &validationErrorMessage{errorResponse, errorMap}, http.StatusUnprocessableEntity)
 }

tequilapi/utils/utils_test.go

@@ -49,7 +49,7 @@ func TestSendErrorRendersErrorMessage(t *testing.T) {
 func TestSendErrorMessageRendersErrorMessage(t *testing.T) {
 	resp := httptest.NewRecorder()
 
-	SendErrorMessage(resp, errorMessage{"error_message"}, http.StatusInternalServerError)
+	SendErrorBody(resp, errorMessage{"error_message"}, http.StatusInternalServerError)
 
 	assert.Equal(t, http.StatusInternalServerError, resp.Code)
 	assert.JSONEq(