rpcsrv: return ErrUnknownSession and ErrUnknownIterator

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:
https://github.com/neo-project/proposals/pull/156
Also adding description of `traverseIterator` in docs/rpc.md.

Signed-off-by: Tatiana Nesterenko <tatiana@nspcc.io>
This commit is contained in:
Tatiana Nesterenko 2023-08-13 21:33:49 +01:00
parent f557959c24
commit f3760c1a98
4 changed files with 49 additions and 30 deletions

View file

@ -196,6 +196,11 @@ enabled in the server's protocol configuration.
##### `getnep11transfers` and `getnep17transfers` ##### `getnep11transfers` and `getnep17transfers`
`transfernotifyindex` is not tracked by NeoGo, thus this field is always zero. `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` ##### `verifyProof`
NeoGo can generate an error in response to an invalid proof, unlike NeoGo can generate an error in response to an invalid proof, unlike

View file

@ -1723,7 +1723,8 @@ func TestClient_IteratorSessions(t *testing.T) {
require.True(t, ok) require.True(t, ok)
ok, err = c.TerminateSession(sID) 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. require.False(t, ok) // session has already been terminated.
}) })
t.Run("automatically", func(t *testing.T) { t.Run("automatically", func(t *testing.T) {
@ -1746,7 +1747,8 @@ func TestClient_IteratorSessions(t *testing.T) {
time.Duration(rpcSrv.config.SessionExpirationTime)*time.Second/4) time.Duration(rpcSrv.config.SessionExpirationTime)*time.Second/4)
ok, err := c.TerminateSession(sID) 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. require.False(t, ok) // session has already been terminated.
}) })
}) })

View file

@ -2351,7 +2351,7 @@ func (s *Server) traverseIterator(reqParams params.Params) (any, *neorpc.Error)
session, ok := s.sessions[sID.String()] session, ok := s.sessions[sID.String()]
if !ok { if !ok {
s.sessionsLock.Unlock() s.sessionsLock.Unlock()
return []json.RawMessage{}, nil return nil, neorpc.ErrUnknownSession
} }
session.iteratorsLock.Lock() session.iteratorsLock.Lock()
// Perform `till` update only after session.iteratorsLock is taken in order to have more // Perform `till` update only after session.iteratorsLock is taken in order to have more
@ -2362,14 +2362,19 @@ func (s *Server) traverseIterator(reqParams params.Params) (any, *neorpc.Error)
var ( var (
iIDStr = iID.String() iIDStr = iID.String()
iVals []stackitem.Item iVals []stackitem.Item
found bool
) )
for _, it := range session.iteratorIdentifiers { for _, it := range session.iteratorIdentifiers {
if iIDStr == it.ID { if iIDStr == it.ID {
iVals = iterator.Values(it.Item, count) iVals = iterator.Values(it.Item, count)
found = true
break break
} }
} }
session.iteratorsLock.Unlock() session.iteratorsLock.Unlock()
if !found {
return nil, neorpc.ErrUnknownIterator
}
result := make([]json.RawMessage, len(iVals)) result := make([]json.RawMessage, len(iVals))
for j := range iVals { for j := range iVals {
@ -2393,7 +2398,9 @@ func (s *Server) terminateSession(reqParams params.Params) (any, *neorpc.Error)
s.sessionsLock.Lock() s.sessionsLock.Lock()
defer s.sessionsLock.Unlock() defer s.sessionsLock.Unlock()
session, ok := s.sessions[strSID] session, ok := s.sessions[strSID]
if ok { if !ok {
return nil, neorpc.ErrUnknownSession
}
// Iterators access Seek channel under the hood; finalizer closes this channel, thus, // Iterators access Seek channel under the hood; finalizer closes this channel, thus,
// we need to perform finalisation under iteratorsLock. // we need to perform finalisation under iteratorsLock.
session.iteratorsLock.Lock() session.iteratorsLock.Lock()
@ -2403,7 +2410,6 @@ func (s *Server) terminateSession(reqParams params.Params) (any, *neorpc.Error)
} }
delete(s.sessions, strSID) delete(s.sessions, strSID)
session.iteratorsLock.Unlock() session.iteratorsLock.Unlock()
}
return ok, nil return ok, nil
} }

View file

@ -2582,6 +2582,13 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) []
return res.Session, *iterator.ID return res.Session, *iterator.ID
} }
t.Run("traverseiterator", func(t *testing.T) { 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) { t.Run("good", func(t *testing.T) {
sID, iID := prepareIteratorSession(t) sID, iID := prepareIteratorSession(t)
expectedCount := 99 expectedCount := 99
@ -2626,36 +2633,35 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) []
_, iID := prepareIteratorSession(t) _, iID := prepareIteratorSession(t)
rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "traverseiterator", "params": ["%s", "%s", %d]}"`, uuid.NewString(), iID.String(), 1) 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) body := doRPCCall(rpc, httpSrv.URL, t)
resp := checkErrGetResult(t, body, false, 0) checkErrGetResult(t, body, true, neorpc.ErrUnknownSessionCode)
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.
}) })
t.Run("unknown iterator", func(t *testing.T) { t.Run("unknown iterator", func(t *testing.T) {
sID, _ := prepareIteratorSession(t) sID, _ := prepareIteratorSession(t)
rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "traverseiterator", "params": ["%s", "%s", %d]}"`, sID.String(), uuid.NewString(), 1) 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) body := doRPCCall(rpc, httpSrv.URL, t)
resp := checkErrGetResult(t, body, false, 0) checkErrGetResult(t, body, true, neorpc.ErrUnknownIteratorCode)
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.
}) })
}) })
t.Run("terminatesession", func(t *testing.T) { t.Run("terminatesession", func(t *testing.T) {
check := func(t *testing.T, id string, expected bool) { rpc := `{"jsonrpc": "2.0", "id": 1, "method": "terminatesession", "params": ["%s"]}"`
rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "terminatesession", "params": ["%s"]}"`, id) t.Run("sessions disabled", func(t *testing.T) {
body := doRPCCall(rpc, httpSrv.URL, 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) resp := checkErrGetResult(t, body, false, 0)
res := new(bool) res := new(bool)
require.NoError(t, json.Unmarshal(resp, res)) require.NoError(t, json.Unmarshal(resp, res))
require.Equal(t, expected, *res) require.Equal(t, true, *res)
}
t.Run("true", func(t *testing.T) {
sID, _ := prepareIteratorSession(t)
check(t, sID.String(), true)
}) })
t.Run("false", func(t *testing.T) { 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) { t.Run("expired", func(t *testing.T) {
_, _ = prepareIteratorSession(t) _, _ = prepareIteratorSession(t)