Skip to content

Commit

Permalink
rpcsrv: return ErrUnknownSession and ErrUnknownIterator
Browse files Browse the repository at this point in the history 
Behaviour change.
`terminatesession` returns ErrUnknownSession in case of impossibility of finding session,
previously there was no-error response with `false` result.
`traverseIterator`returns ErrUnknownSession in case of impossibility of finding session,
previously there was no-error response with default result; `traverseIterator`returns ErrUnknownIterator,
there were no such errors before.
Accordingly to proposal:
neo-project/proposals#156
Also adding description of `traverseIterator` in docs/rpc.md.

Signed-off-by: Tatiana Nesterenko <tatiana@nspcc.io>
  • Loading branch information
@tatiana-nspcc
tatiana-nspcc committed Aug 16, 2023
1 parent f557959 commit f3760c1
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 30 deletions.
5 changes: 5 additions & 0 deletions docs/rpc.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,11 @@ enabled in the server's protocol configuration.
##### `getnep11transfers` and `getnep17transfers`
`transfernotifyindex` is not tracked by NeoGo, thus this field is always zero.

##### `traverseiterator` and `terminatesession`

NeoGo returns an error when it is unable to find a session or iterator, unlike
the error-free C# response that provides a default result.

##### `verifyProof`

NeoGo can generate an error in response to an invalid proof, unlike
Expand Down
6 changes: 4 additions & 2 deletions pkg/services/rpcsrv/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1723,7 +1723,8 @@ func TestClient_IteratorSessions(t *testing.T) {
require.True(t, ok)

ok, err = c.TerminateSession(sID)
require.NoError(t, err)
require.Error(t, err)
require.ErrorIs(t, err, neorpc.ErrUnknownSession)
require.False(t, ok) // session has already been terminated.
})
t.Run("automatically", func(t *testing.T) {
Expand All @@ -1746,7 +1747,8 @@ func TestClient_IteratorSessions(t *testing.T) {
time.Duration(rpcSrv.config.SessionExpirationTime)*time.Second/4)

ok, err := c.TerminateSession(sID)
require.NoError(t, err)
require.Error(t, err)
require.ErrorIs(t, err, neorpc.ErrUnknownSession)
require.False(t, ok) // session has already been terminated.
})
})
Expand Down
28 changes: 17 additions & 11 deletions pkg/services/rpcsrv/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2351,7 +2351,7 @@ func (s *Server) traverseIterator(reqParams params.Params) (any, *neorpc.Error)
session, ok := s.sessions[sID.String()]
if !ok {
s.sessionsLock.Unlock()
return []json.RawMessage{}, nil
return nil, neorpc.ErrUnknownSession
}
session.iteratorsLock.Lock()
// Perform `till` update only after session.iteratorsLock is taken in order to have more
Expand All @@ -2362,14 +2362,19 @@ func (s *Server) traverseIterator(reqParams params.Params) (any, *neorpc.Error)
var (
iIDStr = iID.String()
iVals []stackitem.Item
found bool
)
for _, it := range session.iteratorIdentifiers {
if iIDStr == it.ID {
iVals = iterator.Values(it.Item, count)
found = true
break
}
}
session.iteratorsLock.Unlock()
if !found {
return nil, neorpc.ErrUnknownIterator
}

result := make([]json.RawMessage, len(iVals))
for j := range iVals {
Expand All @@ -2393,17 +2398,18 @@ func (s *Server) terminateSession(reqParams params.Params) (any, *neorpc.Error)
s.sessionsLock.Lock()
defer s.sessionsLock.Unlock()
session, ok := s.sessions[strSID]
if ok {
// Iterators access Seek channel under the hood; finalizer closes this channel, thus,
// we need to perform finalisation under iteratorsLock.
session.iteratorsLock.Lock()
session.finalize()
if !session.timer.Stop() {
<-session.timer.C
}
delete(s.sessions, strSID)
session.iteratorsLock.Unlock()
if !ok {
return nil, neorpc.ErrUnknownSession
}
// Iterators access Seek channel under the hood; finalizer closes this channel, thus,
// we need to perform finalisation under iteratorsLock.
session.iteratorsLock.Lock()
session.finalize()
if !session.timer.Stop() {
<-session.timer.C
}
delete(s.sessions, strSID)
session.iteratorsLock.Unlock()
return ok, nil
}

Expand Down
40 changes: 23 additions & 17 deletions pkg/services/rpcsrv/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2582,6 +2582,13 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) []
return res.Session, *iterator.ID
}
t.Run("traverseiterator", func(t *testing.T) {
t.Run("sessions disabled", func(t *testing.T) {
_, _, httpSrv2 := initClearServerWithCustomConfig(t, func(c *config.Config) {
c.ApplicationConfiguration.RPC.SessionEnabled = false
})
body := doRPCCall(`{"jsonrpc": "2.0", "id": 1, "method": "traverseiterator", "params": []}"`, httpSrv2.URL, t)
checkErrGetResult(t, body, true, neorpc.ErrSessionsDisabledCode)
})
t.Run("good", func(t *testing.T) {
sID, iID := prepareIteratorSession(t)
expectedCount := 99
Expand Down Expand Up @@ -2626,36 +2633,35 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) []
_, iID := prepareIteratorSession(t)
rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "traverseiterator", "params": ["%s", "%s", %d]}"`, uuid.NewString(), iID.String(), 1)
body := doRPCCall(rpc, httpSrv.URL, t)
resp := checkErrGetResult(t, body, false, 0)
res := new([]json.RawMessage)
require.NoError(t, json.Unmarshal(resp, res))
require.Equal(t, 0, len(*res)) // No errors expected, no elements should be returned.
checkErrGetResult(t, body, true, neorpc.ErrUnknownSessionCode)
})
t.Run("unknown iterator", func(t *testing.T) {
sID, _ := prepareIteratorSession(t)
rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "traverseiterator", "params": ["%s", "%s", %d]}"`, sID.String(), uuid.NewString(), 1)
body := doRPCCall(rpc, httpSrv.URL, t)
resp := checkErrGetResult(t, body, false, 0)
res := new([]json.RawMessage)
require.NoError(t, json.Unmarshal(resp, res))
require.Equal(t, 0, len(*res)) // No errors expected, no elements should be returned.
checkErrGetResult(t, body, true, neorpc.ErrUnknownIteratorCode)
})
})
t.Run("terminatesession", func(t *testing.T) {
check := func(t *testing.T, id string, expected bool) {
rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "terminatesession", "params": ["%s"]}"`, id)
body := doRPCCall(rpc, httpSrv.URL, t)
rpc := `{"jsonrpc": "2.0", "id": 1, "method": "terminatesession", "params": ["%s"]}"`
t.Run("sessions disabled", func(t *testing.T) {
_, _, httpSrv2 := initClearServerWithCustomConfig(t, func(c *config.Config) {
c.ApplicationConfiguration.RPC.SessionEnabled = false
})
body := doRPCCall(fmt.Sprintf(rpc, uuid.NewString()), httpSrv2.URL, t)
checkErrGetResult(t, body, true, neorpc.ErrSessionsDisabledCode)
})
t.Run("true", func(t *testing.T) {
sID, _ := prepareIteratorSession(t)
body := doRPCCall(fmt.Sprintf(rpc, sID.String()), httpSrv.URL, t)
resp := checkErrGetResult(t, body, false, 0)
res := new(bool)
require.NoError(t, json.Unmarshal(resp, res))
require.Equal(t, expected, *res)
}
t.Run("true", func(t *testing.T) {
sID, _ := prepareIteratorSession(t)
check(t, sID.String(), true)
require.Equal(t, true, *res)
})
t.Run("false", func(t *testing.T) {
check(t, uuid.NewString(), false)
body := doRPCCall(fmt.Sprintf(rpc, uuid.NewString()), httpSrv.URL, t)
checkErrGetResult(t, body, true, neorpc.ErrUnknownSessionCode)
})
t.Run("expired", func(t *testing.T) {
_, _ = prepareIteratorSession(t)
Expand Down

0 comments on commit f3760c1

Please sign in to comment.