TrueCloudLab/frostfs-sdk-go
33
0
Fork 21
Code Issues 34 Pull requests 1 Releases Activity Actions

[#275] client: Return status from all methods #275

Merged
alexvanin merged 1 commit from mbiryukova/frostfs-sdk-go:bugfix/client_status into master 2024-09-26 10:33:03 +00:00
Conversation 0 Commits 1 Files changed 8 +14 -38
8 changed files with 14 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 1b67ab9608 - Show all commits

2
client/apemanager_list_chains.go
View file

@ -65,7 +65,7 @@ func (c *Client) APEManagerListChains(ctx context.Context, prm PrmAPEManagerList
var res ResAPEManagerListChains
res.st, err = c.processResponse(resp)
if err != nil || !apistatus.IsSuccessful(res.st) {
return nil, err
return &res, err
}
for _, ch := range resp.GetBody().GetChains() {

8
client/netmap.go
View file

@ -239,12 +239,8 @@ func (c *Client) NetMapSnapshot(ctx context.Context, _ PrmNetMapSnapshot) (*ResN
var res ResNetMapSnapshot
res.st, err = c.processResponse(resp)
if err != nil {
return nil, err
}
if !apistatus.IsSuccessful(res.st) {
return &res, nil
if err != nil || !apistatus.IsSuccessful(res.st) {
dkirillov marked this conversation as resolved Outdated
dkirillov commented 2024-09-24 14:24:38 +00:00
Outdated
Review
Copy link

Should we use if err != nil || !apistatus.IsSuccessful(res.st) ? Probably we need someone from core team

Should we use `if err != nil || !apistatus.IsSuccessful(res.st)` ? Probably we need someone from `core` team
dkirillov commented 2024-09-24 14:26:02 +00:00
Outdated
Review
Copy link

Oh, I see next line (246). So suppose we can freely use if err != nil || !apistatus.IsSuccessful(res.st) and drop lines 246-248

Oh, I see next line (246). So suppose we can freely use `if err != nil || !apistatus.IsSuccessful(res.st)` and drop lines 246-248
👍 1
return &res, err
}
const fieldNetMap = "network map"

8
client/object_delete.go
View file

@ -148,12 +148,8 @@ func (c *Client) ObjectDelete(ctx context.Context, prm PrmObjectDelete) (*ResObj
var res ResObjectDelete
res.st, err = c.processResponse(resp)
if err != nil {
return nil, err
}
if !apistatus.IsSuccessful(res.st) {
return &res, nil
if err != nil || !apistatus.IsSuccessful(res.st) {
return &res, err
}
const fieldTombstone = "tombstone"

8
client/object_get.go
View file

@ -492,12 +492,8 @@ func (c *Client) ObjectHead(ctx context.Context, prm PrmObjectHead) (*ResObjectH
var res ResObjectHead
res.st, err = c.processResponse(resp)
if err != nil {
return nil, err
}
if !apistatus.IsSuccessful(res.st) {
return &res, nil
if err != nil || !apistatus.IsSuccessful(res.st) {
return &res, err
}
res.idObj = *prm.ObjectID

8
client/object_hash.go
View file

@ -189,12 +189,8 @@ func (c *Client) ObjectHash(ctx context.Context, prm PrmObjectHash) (*ResObjectH
var res ResObjectHash
res.st, err = c.processResponse(resp)
if err != nil {
return nil, err
}
if !apistatus.IsSuccessful(res.st) {
return &res, nil
if err != nil || !apistatus.IsSuccessful(res.st) {
return &res, err
}
res.checksums = resp.GetBody().GetHashList()

8
client/object_patch.go
View file

@ -246,12 +246,8 @@ func (x *objectPatcher) Close(_ context.Context) (*ResObjectPatch, error) {
}
x.res.st, x.err = x.client.processResponse(&x.respV2)
if x.err != nil {
return nil, x.err
}
if !apistatus.IsSuccessful(x.res.st) {
return &x.res, nil
if x.err != nil || !apistatus.IsSuccessful(x.res.st) {
return &x.res, x.err
}
const fieldID = "ID"

8
client/object_put_raw.go
View file

@ -156,12 +156,8 @@ func (x *objectWriterRaw) Close(_ context.Context) (*ResObjectPut, error) {
}
x.res.st, x.err = x.client.processResponse(&x.respV2)
if x.err != nil {
return nil, x.err
}
if !apistatus.IsSuccessful(x.res.st) {
return &x.res, nil
if x.err != nil || !apistatus.IsSuccessful(x.res.st) {
aarifullin commented 2024-09-25 12:58:42 +00:00
Review
Copy link

It seems that then the errors may not be distinguished: either the error occurred by performing a request (signature check failure) or by processing of request (APE check failure while PutObject handling).
Although, we have such an error resolution:

func (c *Client) processResponse(resp responseV2) (apistatus.Status, error) {
	if c.prm.ResponseInfoCallback != nil {
		rmi := ResponseMetaInfo{
			key:   resp.GetVerificationHeader().GetBodySignature().GetKey(),
			epoch: resp.GetMetaHeader().GetEpoch(),
		}
		if err := c.prm.ResponseInfoCallback(rmi); err != nil {
			return nil, fmt.Errorf("response callback error: %w", err)
		}
	}

	err := signature.VerifyServiceMessage(resp)
	if err != nil {
		return nil, fmt.Errorf("invalid response signature: %w", err)
	}

	st := apistatus.FromStatusV2(resp.GetMetaHeader().GetStatus())
	if !c.prm.DisableFrostFSErrorResolution {
		return st, apistatus.ErrFromStatus(st)
	}
	return st, nil
}

that doesn't seem controversial to the introduced approach

It seems that then the errors may **not** be distinguished: either the error occurred by _performing_ a request (signature check failure) or by _processing_ of request (APE check failure while `PutObject` handling). Although, we have such an error resolution: ```go func (c *Client) processResponse(resp responseV2) (apistatus.Status, error) { if c.prm.ResponseInfoCallback != nil { rmi := ResponseMetaInfo{ key: resp.GetVerificationHeader().GetBodySignature().GetKey(), epoch: resp.GetMetaHeader().GetEpoch(), } if err := c.prm.ResponseInfoCallback(rmi); err != nil { return nil, fmt.Errorf("response callback error: %w", err) } } err := signature.VerifyServiceMessage(resp) if err != nil { return nil, fmt.Errorf("invalid response signature: %w", err) } st := apistatus.FromStatusV2(resp.GetMetaHeader().GetStatus()) if !c.prm.DisableFrostFSErrorResolution { return st, apistatus.ErrFromStatus(st) } return st, nil } ``` that doesn't seem controversial to the introduced approach
return &x.res, x.err
}
const fieldID = "ID"

2
client/object_put_single.go
View file

@ -167,7 +167,7 @@ func (c *Client) ObjectPutSingle(ctx context.Context, prm PrmObjectPutSingle) (*
var res ResObjectPutSingle
res.st, err = c.processResponse(resp)
if err != nil {
return nil, err
return &res, err
}
res.epoch = resp.GetMetaHeader().GetEpoch()