Skip to content
This repository has been archived by the owner on Dec 23, 2024. It is now read-only.

A problem with decoding vshard storage answers #42

Closed
nurzhan-saktaganov opened this issue Aug 19, 2024 · 3 comments · Fixed by #68
Closed

A problem with decoding vshard storage answers #42

nurzhan-saktaganov opened this issue Aug 19, 2024 · 3 comments · Fixed by #68
Labels
bug Something isn't working

Comments

@nurzhan-saktaganov
Copy link
Collaborator

nurzhan-saktaganov commented Aug 19, 2024

Problem 1

RouterMapCallRWImpl lacks checks for len(respData) before addressing its elements.

For example, here

if respData[0] == nil {

and here

err = mapstructure.Decode(respData[1], vshardErr)

and so on...

Problem 2

In ReplicaCall, we consider an error if len(respData) != 2. But, if fnc is completed successfully but does not return anything (i.e. returns nil), vshard does not send the second element (i.e. does not send nil):

if len(respData) != 2 {

The fix is expected to be similar to PR #35.

@nurzhan-saktaganov
Copy link
Collaborator Author

The same problem here:

err := future.GetTyped(&[]interface{}{&struct {

@nurzhan-saktaganov nurzhan-saktaganov added the bug Something isn't working label Aug 26, 2024
@nurzhan-saktaganov
Copy link
Collaborator Author

There is another more recommended patch from colleague

From 58a807944e510f0fe403fc081e10f98f01ebe726 Mon Sep 17 00:00:00 2001
From: Vladimir Patikov <v.patikov@corp.mail.ru>
Date: Tue, 3 Sep 2024 17:32:50 +0300
Subject: [PATCH] ReplicaCall response parsing fix

---
 replicaset.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/replicaset.go b/replicaset.go
index e7f4a00..d5e880f 100644
--- a/replicaset.go
+++ b/replicaset.go
@@ -119,12 +119,12 @@ func (rs *Replicaset) ReplicaCall(
 			continue
 		}
 
-		if len(respData) != 2 {
-			err = fmt.Errorf("invalid length of response data: must be = 2, current: %d", len(respData))
+		if len(respData) < 1 {
+			err = fmt.Errorf("response data is empty")
 			continue
 		}
 
-		if respData[1] != nil {
+		if len(respData) > 1 && respData[1] != nil {
 			assertErr := &StorageCallAssertError{}
 
 			err = mapstructure.Decode(respData[1], assertErr)
-- 
1.8.3.1

nurzhan-saktaganov added a commit that referenced this issue Sep 7, 2024
* RouterCallImpl: fix decoding responce from storage_ref
* RouterCallImpl: fix decoding responce from storage_map
* BucketDiscovery: check res for nil
* BucketStat: decode bsInfo by ptr
* Add tnt tests for disvoery logic
* Add tnt tests for RouterCallImpl
* Add tnt tests for RouterMapCallRWImpl
nurzhan-saktaganov added a commit that referenced this issue Sep 12, 2024
* RouterCallImpl: fix decoding responce from storage_ref
* RouterCallImpl: fix decoding responce from storage_map
* BucketDiscovery: check res for nil
* BucketStat: decode bsInfo by ptr
* Add tnt tests for disvoery logic
* Add tnt tests for RouterCallImpl
* Add tnt tests for RouterMapCallRWImpl
nurzhan-saktaganov added a commit that referenced this issue Sep 12, 2024
* RouterCallImpl: fix decoding responce from storage_ref
* RouterCallImpl: fix decoding responce from storage_map
* BucketDiscovery: check res for nil
* BucketStat: decode bsInfo by ptr
* Add tnt tests for disvoery logic
* Add tnt tests for RouterCallImpl
* Add tnt tests for RouterMapCallRWImpl
nurzhan-saktaganov added a commit that referenced this issue Sep 12, 2024
* RouterCallImpl: fix decoding response from storage_ref
* RouterCallImpl: fix decoding response from storage_map
* BucketDiscovery: check res for nil
* BucketStat: decode bsInfo by ptr
* Add tnt tests for discovery logic
* Add tnt tests for RouterCallImpl
* Add tnt tests for RouterMapCallRWImpl
* Add tnt tests for topology logic
This was linked to pull requests Sep 13, 2024
This was unlinked from pull requests Sep 13, 2024
@nurzhan-saktaganov
Copy link
Collaborator Author

We still have decoding problems in func (rs *Replicaset) ReplicaCall, there will be another PR with solution

nurzhan-saktaganov added a commit that referenced this issue Sep 14, 2024
* ReplicaCall
	- fix decoding response (#42)
	- fix ignoring timeout while waiting for future.Get()
* Introduce Replicaset.CallAsync and struct Future
* Add tests for methods of Replicaset and Future
nurzhan-saktaganov added a commit that referenced this issue Sep 14, 2024
* ReplicaCall
	- fix decoding response (#42)
	- fix ignoring timeout while waiting for future.Get()
* Introduce Replicaset.CallAsync and struct Future
* Add tests for methods of Replicaset and Future
nurzhan-saktaganov added a commit that referenced this issue Sep 17, 2024
* ReplicaCall
	- fix decoding response (#42)
	- fix ignoring timeout while waiting for future.Get()
* Introduce Replicaset.CallAsync and struct Future
* Add tests for methods of Replicaset and Future
nurzhan-saktaganov added a commit that referenced this issue Sep 17, 2024
* ReplicaCall
	- fix decoding response (#42)
	- fix ignoring timeout while waiting for future.Get()
* Introduce Replicaset.CallAsync
* Add tests for methods of Replicaset
KaymeKaydex pushed a commit that referenced this issue Sep 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant