BUG: Set function in NB returns a response even if an error occurs

[adibrastegarnia]

Set function returns a response even if an error occurs in NB side. For example, if we send an invalid Set request to the NB, the NB returns the same response as the request is valid. We need to do error checking before sending the response back to the client and if an error occurs we should return an error message using the gnmi extension message.

[SeanCondon]

Can you be more specific about what you mean with an invalid Set. there are several cases in which error is thrown. Also I don't think we should use the extension for the error - we just return an error like line 157 of set.go.

[adibrastegarnia]

Suppose you provide a wrong path (e.g. change something in the request to make the path in the Path tree wrong, e.g. System to System2). As far as I know, the Error message which was part of the specification originally is deprecated and we should use extension messages to report the error back to the client. There are different types of error that we need to handle and the best way to do it is to include the error message in an extension message (we can use the RegisteredExtension for that purpose and we don't need to define one).

As a reference:

message SetResponse { Path prefix = 1; // Prefix used for paths. // A set of responses specifying the result of the operations specified in // the SetRequest. repeated UpdateResult response = 2; Error message = 3 [deprecated=true]; // The overall status of the transaction. int64 timestamp = 4; // Timestamp of transaction (ns since epoch). // Extension messages associated with the SetResponse. See the // gNMI extension specification for further definition. repeated gnmi_ext.Extension extension = 5; }

P.S We can return an error as well but if we want to include the error part of the response we need to use extension message.

[SeanCondon]

Again - please see line 157 of set.go - it returns a gRPC error with the given text - and not a SetResponse - and so no extension either

I say this because of statement on line 173 of gnmi.proto: // Error message previously utilised to return errors to the client. Deprecated // in favour of using the google.golang.org/genproto/googleapis/rpc/status // message in the RPC response. // Reference: gNMI Specification Section 2.5

[adibrastegarnia]

Yes. That is true. We should return the error directory without encapsulating it in a message.

[adibrastegarnia]

@SeanCondon Did you reproduce it or should I send you an example if it is needed?

[SeanCondon]

Regarding the wrong path - there will be future work to support YANG models to prevent wrong paths. This is covered by #98

[adibrastegarnia]

Closed since it is covered by #98