From f88b8a2e25b9a5d915ef027e29f6e3f22d5547ab Mon Sep 17 00:00:00 2001 From: Alexander North Date: Fri, 24 Jul 2026 15:00:13 +0200 Subject: [PATCH] add new not connected error type and grpc unavailable mapping --- pkg/datastore/intent_rpc.go | 2 +- pkg/datastore/target/gnmi/gnmi.go | 2 +- pkg/datastore/target/netconf/nc.go | 4 +- pkg/datastore/target/types/targetstatus.go | 6 ++ pkg/server/transaction.go | 9 ++- pkg/server/transaction_test.go | 70 ++++++++++++++++++++++ 6 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 pkg/server/transaction_test.go diff --git a/pkg/datastore/intent_rpc.go b/pkg/datastore/intent_rpc.go index 974384d5..f9a3b48b 100644 --- a/pkg/datastore/intent_rpc.go +++ b/pkg/datastore/intent_rpc.go @@ -46,7 +46,7 @@ func (d *Datastore) applyIntent(ctx context.Context, source targettypes.TargetSo } if d.sbi == nil { - return nil, fmt.Errorf("%s is not connected", d.config.Name) + return nil, fmt.Errorf("%s: %w", d.config.Name, targettypes.ErrNotConnected) } rsp, err = d.sbi.Set(ctx, source) diff --git a/pkg/datastore/target/gnmi/gnmi.go b/pkg/datastore/target/gnmi/gnmi.go index a174af9a..7df85f5f 100644 --- a/pkg/datastore/target/gnmi/gnmi.go +++ b/pkg/datastore/target/gnmi/gnmi.go @@ -161,7 +161,7 @@ func (t *gnmiTarget) Set(ctx context.Context, source targetTypes.TargetSource) ( var err error if t == nil { - return nil, fmt.Errorf("%s", "not connected") + return nil, targetTypes.ErrNotConnected } // deletes from protos diff --git a/pkg/datastore/target/netconf/nc.go b/pkg/datastore/target/netconf/nc.go index e04d9775..064eda2b 100644 --- a/pkg/datastore/target/netconf/nc.go +++ b/pkg/datastore/target/netconf/nc.go @@ -99,7 +99,7 @@ func (t *ncTarget) internalGet(ctx context.Context, req *sdcpb.GetDataRequest) ( ctx = logf.IntoContext(ctx, log) if !t.Status().IsConnected() { - return nil, fmt.Errorf("%s", types.TargetStatusNotConnected) + return nil, fmt.Errorf("%s: %w", t.name, types.ErrNotConnected) } source := "running" @@ -163,7 +163,7 @@ func (t *ncTarget) Set(ctx context.Context, source types.TargetSource) (*sdcpb.S log := logf.FromContext(ctx).WithName("Set") ctx = logf.IntoContext(ctx, log) if !t.Status().IsConnected() { - return nil, fmt.Errorf("%s", types.TargetStatusNotConnected) + return nil, fmt.Errorf("%s: %w", t.name, types.ErrNotConnected) } switch t.sbiConfig.NetconfOptions.CommitDatastore { diff --git a/pkg/datastore/target/types/targetstatus.go b/pkg/datastore/target/types/targetstatus.go index 249f8767..6fb903a6 100644 --- a/pkg/datastore/target/types/targetstatus.go +++ b/pkg/datastore/target/types/targetstatus.go @@ -1,5 +1,11 @@ package types +import "errors" + +// ErrNotConnected indicates the southbound interface (device connection) of a +// datastore is not established +var ErrNotConnected = errors.New("not connected") + type TargetStatus struct { Status TargetConnectionStatus Details string diff --git a/pkg/server/transaction.go b/pkg/server/transaction.go index 4e8be390..89e9f25f 100644 --- a/pkg/server/transaction.go +++ b/pkg/server/transaction.go @@ -7,6 +7,7 @@ import ( "time" "github.com/sdcio/data-server/pkg/datastore" + targettypes "github.com/sdcio/data-server/pkg/datastore/target/types" "github.com/sdcio/data-server/pkg/datastore/types" "github.com/sdcio/data-server/pkg/tree/consts" "github.com/sdcio/data-server/pkg/utils" @@ -153,8 +154,12 @@ func (s *Server) TransactionCancel(ctx context.Context, req *sdcpb.TransactionCa // translateInternalToGrpcError central function to map internal errors to grpc error codes func translateInternalToGrpcError(err error) error { - if errors.Is(err, datastore.ErrDatastoreLocked) { + switch { + case errors.Is(err, datastore.ErrDatastoreLocked): return status.Error(codes.Aborted, err.Error()) + case errors.Is(err, targettypes.ErrNotConnected): + return status.Error(codes.Unavailable, err.Error()) + default: + return err } - return err } diff --git a/pkg/server/transaction_test.go b/pkg/server/transaction_test.go new file mode 100644 index 00000000..44361dcc --- /dev/null +++ b/pkg/server/transaction_test.go @@ -0,0 +1,70 @@ +package server + +import ( + "errors" + "fmt" + "testing" + + "github.com/sdcio/data-server/pkg/datastore" + targettypes "github.com/sdcio/data-server/pkg/datastore/target/types" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestTranslateInternalToGrpcError(t *testing.T) { + cases := map[string]struct { + err error + wantNil bool + wantSame bool // returned unchanged (non-status passthrough) + wantCode codes.Code + }{ + "nil": { + err: nil, + wantNil: true, + }, + "datastore locked -> Aborted": { + err: datastore.ErrDatastoreLocked, + wantCode: codes.Aborted, + }, + "not connected -> Unavailable": { + err: targettypes.ErrNotConnected, + wantCode: codes.Unavailable, + }, + "wrapped not connected -> Unavailable": { + err: fmt.Errorf("some.datastore: %w", targettypes.ErrNotConnected), + wantCode: codes.Unavailable, + }, + "other error -> passthrough": { + err: errors.New("boom"), + wantSame: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := translateInternalToGrpcError(tc.err) + + if tc.wantNil { + if got != nil { + t.Fatalf("expected nil, got %v", got) + } + return + } + + if tc.wantSame { + if got != tc.err { + t.Fatalf("expected passthrough of the same error, got %v", got) + } + return + } + + st, ok := status.FromError(got) + if !ok { + t.Fatalf("expected a gRPC status error, got %v", got) + } + if st.Code() != tc.wantCode { + t.Fatalf("expected code %v, got %v (%v)", tc.wantCode, st.Code(), got) + } + }) + } +}