Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Commit

Permalink
Categorize session restoration errors
Browse files Browse the repository at this point in the history
Dedicate special reply message types for "session not found"
and "session reached max connections" errors.
Add test to verify the correct error type is returned during
restoration of an inexistent session.
  • Loading branch information
romshark committed Mar 18, 2018
1 parent 3cfcddf commit ac73199
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 21 deletions.
12 changes: 12 additions & 0 deletions client/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ func (clt *Client) handleReplyShutdown(reqIdent [8]byte) {
clt.requestManager.Fail(reqIdent, webwire.ReqErrSrvShutdown{})
}

func (clt *Client) handleSessionNotFound(reqIdent [8]byte) {
clt.requestManager.Fail(reqIdent, webwire.SessNotFound{})
}

func (clt *Client) handleMaxSessConnsReached(reqIdent [8]byte) {
clt.requestManager.Fail(reqIdent, webwire.MaxSessConnsReached{})
}

func (clt *Client) handleReply(reqID [8]byte, payload webwire.Payload) {
clt.requestManager.Fulfill(reqID, payload)
}
Expand Down Expand Up @@ -85,6 +93,10 @@ func (clt *Client) handleMessage(message []byte) error {
)
case webwire.MsgReplyShutdown:
clt.handleReplyShutdown(extractMessageIdentifier(message))
case webwire.MsgSessionNotFound:
clt.handleSessionNotFound(extractMessageIdentifier(message))
case webwire.MsgMaxSessConnsReached:
clt.handleMaxSessConnsReached(extractMessageIdentifier(message))
case webwire.MsgErrorReply:
clt.handleFailure(extractMessageIdentifier(message), message[9:])
case webwire.MsgReplyInternalError:
Expand Down
6 changes: 2 additions & 4 deletions client/requestSessionRestoration.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

// requestSessionRestoration sends a session restoration request
// and decodes the session object from the received replied.
// and decodes the session object from the received reply.
// Expects the client to be connected beforehand
func (clt *Client) requestSessionRestoration(sessionKey []byte) (*webwire.Session, error) {
reply, err := clt.sendNamelessRequest(
Expand All @@ -20,9 +20,7 @@ func (clt *Client) requestSessionRestoration(sessionKey []byte) (*webwire.Sessio
clt.defaultTimeout,
)
if err != nil {
return nil, webwire.NewReqErrTrans(
fmt.Errorf("Session restoration request failed: %s", err),
)
return nil, err
}

var session webwire.Session
Expand Down
16 changes: 16 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,19 @@ type ReqErr struct {
func (err ReqErr) Error() string {
return err.Message
}

// SessNotFound represents a session restoration error type indicating that the server didn't
// find the session to be restored
type SessNotFound struct{}

func (err SessNotFound) Error() string {
return "Session not found"
}

// MaxSessConnsReached represents an authentication error type indicating that the given session
// already reched the maximum number of concurrent connections
type MaxSessConnsReached struct{}

func (err MaxSessConnsReached) Error() string {
return "Reached maximum number of concurrent session connections"
}
12 changes: 12 additions & 0 deletions message.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ const (
// the processing of a request
MsgReplyInternalError = byte(2)

// MsgSessionNotFound is sent by the server in response to an unfilfilled session restoration
// request due to the session not being found
MsgSessionNotFound = byte(3)

// MsgMaxSessConnsReached is sent by the server in response to an authentication request
// when the maximum number of concurrent connections for a certain session was reached
MsgMaxSessConnsReached = byte(4)

// MsgSessionCreated is sent by the server
// to notify the client about the session creation
MsgSessionCreated = byte(21)
Expand Down Expand Up @@ -774,6 +782,10 @@ func (msg *Message) createFailCallback(client *Client, srv *Server) {
} else {
report = []byte(`{"c":""}`)
}
case MaxSessConnsReached:
msgType = MsgMaxSessConnsReached
case SessNotFound:
msgType = MsgSessionNotFound
default:
msgType = MsgReplyInternalError
}
Expand Down
1 change: 0 additions & 1 deletion payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ func (pld *Payload) Utf8() (string, error) {
return "", fmt.Errorf("Cannot convert invalid UTF16 payload data to UTF8")
}
u16str := make([]uint16, 1)
// TODO: check whether the builtin append would be more efficient than bytes.Buffer
utf8str := &bytes.Buffer{}
utf8buf := make([]byte, 4)
for i := 0; i < len(pld.Data); i += 2 {
Expand Down
16 changes: 2 additions & 14 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,7 @@ func (srv *Server) handleSessionRestore(msg *Message) error {

if srv.SessionRegistry.maxConns > 0 &&
srv.SessionRegistry.SessionConnections(key)+1 > srv.SessionRegistry.maxConns {
// TODO: Implement dedicated error message type for "max connection reached" errors
msg.fail(ReqErr{
Code: "MAX_CONN_REACHED",
Message: fmt.Sprintf(
"Session %s reached the maximum number of concurrent connections (%d)",
key,
srv.SessionRegistry.maxConns,
),
})
msg.fail(MaxSessConnsReached{})
return nil
}

Expand All @@ -224,11 +216,7 @@ func (srv *Server) handleSessionRestore(msg *Message) error {
return fmt.Errorf("CRITICAL: Session search handler failed: %s", err)
}
if session == nil {
// TODO: Implement dedicated error message type for "session not found" errors
msg.fail(ReqErr{
Code: "SESSION_NOT_FOUND",
Message: fmt.Sprintf("No session associated with key: '%s'", key),
})
msg.fail(SessNotFound{})
return nil
}

Expand Down
10 changes: 8 additions & 2 deletions test/maxConcSessConn_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package test

import (
"reflect"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -91,8 +92,13 @@ func TestMaxConcSessConn(t *testing.T) {

// Try to restore the session and expect this operation to fail due to reached limit
sessionKeyLock.RLock()
if err := superflousClient.RestoreSession([]byte(sessionKey)); err == nil {
t.Fatalf("Expected an error during superfluous client manual session restoration")
sessRestErr := superflousClient.RestoreSession([]byte(sessionKey))
if _, isMaxReachedErr := sessRestErr.(wwr.MaxSessConnsReached); !isMaxReachedErr {
t.Fatalf(
"Expected a MaxSessConnsReached error during manual session restoration, got: %s | %s",
reflect.TypeOf(sessRestErr),
sessRestErr,
)
}
sessionKeyLock.RUnlock()
}
58 changes: 58 additions & 0 deletions test/restoreInexistentSession_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package test

import (
"reflect"
"testing"
"time"

wwr "github.com/qbeon/webwire-go"
wwrClient "github.com/qbeon/webwire-go/client"
)

// TestRestoreInexistentSession tests the restoration of an inexistent session
func TestRestoreInexistentSession(t *testing.T) {
// Initialize server
_, addr := setupServer(
t,
wwr.ServerOptions{
Hooks: wwr.Hooks{
// Permanently store the session
OnSessionCreated: func(_ *wwr.Client) error {
return nil
},
// Find session by key
OnSessionLookup: func(_ string) (*wwr.Session, error) {
return nil, nil
},
// Define dummy hook to enable sessions on this server
OnSessionClosed: func(_ *wwr.Client) error {
return nil
},
},
},
)

// Initialize client

// Ensure that the last superfluous client is rejected
client := wwrClient.NewClient(
addr,
wwrClient.Options{
DefaultRequestTimeout: 2 * time.Second,
},
)

if err := client.Connect(); err != nil {
t.Fatalf("Couldn't connect client: %s", err)
}

// Try to restore the session and expect it to fail due to the session being inexistent
sessRestErr := client.RestoreSession([]byte("lalala"))
if _, isSessNotFoundErr := sessRestErr.(wwr.SessNotFound); !isSessNotFoundErr {
t.Fatalf(
"Expected a SessNotFound error during manual session restoration, got: %s | %s",
reflect.TypeOf(sessRestErr),
sessRestErr,
)
}
}

0 comments on commit ac73199

Please sign in to comment.