From 6ab58cb341e654aaf10b891cecd2017bc8589cd1 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Tue, 10 Apr 2018 13:23:51 +0300 Subject: [PATCH 1/4] Return 499 when connection creation is cancelled with Cancelable --- client/connection/manager.go | 2 +- tequilapi/endpoints/connection.go | 3 +++ tequilapi/endpoints/connection_test.go | 27 ++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/client/connection/manager.go b/client/connection/manager.go index 772930952..beb4aac76 100644 --- a/client/connection/manager.go +++ b/client/connection/manager.go @@ -64,7 +64,7 @@ func (manager *connectionManager) Connect(consumerID, providerID identity.Identi 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() }) diff --git a/tequilapi/endpoints/connection.go b/tequilapi/endpoints/connection.go index d60183963..389e95a5b 100644 --- a/tequilapi/endpoints/connection.go +++ b/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" ) @@ -73,6 +74,8 @@ func (ce *ConnectionEndpoint) Create(resp http.ResponseWriter, req *http.Request utils.SendError(resp, err, http.StatusConflict) case connection.ErrConnectionCancelled: utils.SendError(resp, err, statusConnectCancelled) + case node_utils.ErrRequestCancelled: + utils.SendError(resp, connection.ErrConnectionCancelled, statusConnectCancelled) default: log.Error(connectionLogPrefix, err) utils.SendError(resp, err, http.StatusInternalServerError) diff --git a/tequilapi/endpoints/connection_test.go b/tequilapi/endpoints/connection_test.go index deb13efb8..8582c793d 100644 --- a/tequilapi/endpoints/connection_test.go +++ b/tequilapi/endpoints/connection_test.go @@ -438,3 +438,30 @@ func TestConnectReturnsConnectCancelledStatusWhenErrConnectionCancelledIsEncount resp.Body.String(), ) } + +func TestConnectReturnsConnectCancelledStatusWhenErrRequestCancelledIsEncountered(t *testing.T) { + manager := fakeManager{} + manager.onConnectReturn = utils.ErrRequestCancelled + + connectionEndpoint := NewConnectionEndpoint(&manager, nil, nil) + req := httptest.NewRequest( + http.MethodPut, + "/irrelevant", + strings.NewReader( + `{ + "consumerId" : "my-identity", + "providerId" : "required-node" + }`)) + resp := httptest.NewRecorder() + + connectionEndpoint.Create(resp, req, nil) + + assert.Equal(t, statusConnectCancelled, resp.Code) + assert.JSONEq( + t, + `{ + "message" : "connection was cancelled" + }`, + resp.Body.String(), + ) +} From 06d60489331ccd4682b832097d9b2cba981d23bc Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Tue, 10 Apr 2018 13:25:24 +0300 Subject: [PATCH 2/4] Fix triggered linter errors --- tequilapi/endpoints/connection_test.go | 2 +- tequilapi/endpoints/health_check_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tequilapi/endpoints/connection_test.go b/tequilapi/endpoints/connection_test.go index 8582c793d..289b8542d 100644 --- a/tequilapi/endpoints/connection_test.go +++ b/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 } diff --git a/tequilapi/endpoints/health_check_test.go b/tequilapi/endpoints/health_check_test.go index 9cb283438..28e9e5fcb 100644 --- a/tequilapi/endpoints/health_check_test.go +++ b/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 } From b73933f0deb419c0af4762cdbf55da20aa525f26 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Wed, 11 Apr 2018 15:52:05 +0300 Subject: [PATCH 3/4] Return single cancel error object in Connect --- client/connection/manager.go | 10 +++++++++- tequilapi/endpoints/connection.go | 3 --- tequilapi/endpoints/connection_test.go | 27 -------------------------- 3 files changed, 9 insertions(+), 31 deletions(-) diff --git a/client/connection/manager.go b/client/connection/manager.go index beb4aac76..6f15324b0 100644 --- a/client/connection/manager.go +++ b/client/connection/manager.go @@ -62,6 +62,14 @@ func (manager *connectionManager) Connect(consumerID, providerID identity.Identi } }() + err = manager.startConnection(consumerID, providerID) + if err == utils.ErrRequestCancelled { + return ErrConnectionCancelled + } + return err +} + +func (manager *connectionManager) startConnection(consumerID, providerID identity.Identity) (err error) { cancelable := utils.NewCancelable() manager.cleanConnection = utils.CallOnce(func() { log.Info(managerLogPrefix, "Cancelling connection initiation") @@ -214,7 +222,7 @@ func (manager *connectionManager) waitForConnectedState(stateChannel <-chan open manager.onStateChanged(state, sessionID) } case <-cancelRequest: - return ErrConnectionCancelled + return utils.ErrRequestCancelled } } } diff --git a/tequilapi/endpoints/connection.go b/tequilapi/endpoints/connection.go index 389e95a5b..d60183963 100644 --- a/tequilapi/endpoints/connection.go +++ b/tequilapi/endpoints/connection.go @@ -10,7 +10,6 @@ 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" ) @@ -74,8 +73,6 @@ func (ce *ConnectionEndpoint) Create(resp http.ResponseWriter, req *http.Request utils.SendError(resp, err, http.StatusConflict) case connection.ErrConnectionCancelled: utils.SendError(resp, err, statusConnectCancelled) - case node_utils.ErrRequestCancelled: - utils.SendError(resp, connection.ErrConnectionCancelled, statusConnectCancelled) default: log.Error(connectionLogPrefix, err) utils.SendError(resp, err, http.StatusInternalServerError) diff --git a/tequilapi/endpoints/connection_test.go b/tequilapi/endpoints/connection_test.go index 289b8542d..fcb530d92 100644 --- a/tequilapi/endpoints/connection_test.go +++ b/tequilapi/endpoints/connection_test.go @@ -438,30 +438,3 @@ func TestConnectReturnsConnectCancelledStatusWhenErrConnectionCancelledIsEncount resp.Body.String(), ) } - -func TestConnectReturnsConnectCancelledStatusWhenErrRequestCancelledIsEncountered(t *testing.T) { - manager := fakeManager{} - manager.onConnectReturn = utils.ErrRequestCancelled - - connectionEndpoint := NewConnectionEndpoint(&manager, nil, nil) - req := httptest.NewRequest( - http.MethodPut, - "/irrelevant", - strings.NewReader( - `{ - "consumerId" : "my-identity", - "providerId" : "required-node" - }`)) - resp := httptest.NewRecorder() - - connectionEndpoint.Create(resp, req, nil) - - assert.Equal(t, statusConnectCancelled, resp.Code) - assert.JSONEq( - t, - `{ - "message" : "connection was cancelled" - }`, - resp.Body.String(), - ) -} From 4d1f01a8bdd9ac1b3ea99c849188b159408ec9df Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Wed, 11 Apr 2018 17:12:22 +0300 Subject: [PATCH 4/4] Remove ErrConnectionCancelled error --- client/connection/manager.go | 8 +------- client/connection/manager_test.go | 3 ++- tequilapi/endpoints/connection.go | 5 +++-- tequilapi/endpoints/connection_test.go | 2 +- tequilapi/utils/utils.go | 13 +++++++++---- tequilapi/utils/utils_test.go | 2 +- 6 files changed, 17 insertions(+), 16 deletions(-) diff --git a/client/connection/manager.go b/client/connection/manager.go index 6f15324b0..1073972fd 100644 --- a/client/connection/manager.go +++ b/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,11 +60,7 @@ func (manager *connectionManager) Connect(consumerID, providerID identity.Identi } }() - err = manager.startConnection(consumerID, providerID) - if err == utils.ErrRequestCancelled { - return ErrConnectionCancelled - } - return err + return manager.startConnection(consumerID, providerID) } func (manager *connectionManager) startConnection(consumerID, providerID identity.Identity) (err error) { diff --git a/client/connection/manager_test.go b/client/connection/manager_test.go index c62c933c5..74f8d21fc 100644 --- a/client/connection/manager_test.go +++ b/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() { diff --git a/tequilapi/endpoints/connection.go b/tequilapi/endpoints/connection.go index d60183963..dbad3d30f 100644 --- a/tequilapi/endpoints/connection.go +++ b/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) diff --git a/tequilapi/endpoints/connection_test.go b/tequilapi/endpoints/connection_test.go index fcb530d92..1259a39cc 100644 --- a/tequilapi/endpoints/connection_test.go +++ b/tequilapi/endpoints/connection_test.go @@ -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( diff --git a/tequilapi/utils/utils.go b/tequilapi/utils/utils.go index d3eeb39c5..98db3f660 100644 --- a/tequilapi/utils/utils.go +++ b/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) } diff --git a/tequilapi/utils/utils_test.go b/tequilapi/utils/utils_test.go index 78aa02f21..c331b5bd0 100644 --- a/tequilapi/utils/utils_test.go +++ b/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(