Describe errors that are to be "expected"

[SebastianSchildt]

We sort of have the (unwritten?) alignment that error codes/reasons should be "aligned to VISS", we should document which errors are to be expected in the kuksa.val.v1 API under which conditions on the individual API calls

Should likely be markup in this repo, or - if there is some standard for that - could be documented inside the proto files

[SebastianSchildt]

Related: #560

[erikbosch]

Can be integrated with #390

[erikbosch]

I created a wiki-page to describe current status at https://github.com/eclipse/kuksa.val/wiki/KUKSA.val-gRPC-interfaces. I suggest we first document current status on wiki, then we can decide what changes that are needed in the short term so that we create "real" documentation first when the obvious gaps have been fixed

[sophokles73]

@erikbosch I have read the wiki page you have created and I believe it is a good start. I have some comments/questions but I am not sure where/how to post them. Do you want me to fork the wiki repo and create a PR? Or would you rather have me cite the corresponding sections here in this issue?

[erikbosch]

I assume you do not have access to edit it, otherwise an option could be to add discussion points directly in it. Forking/copying it and highlight problems or proposed changes and reference that from this issue could possibly be an idea.

[sophokles73]

Error handling (currently)

Get and Set RPCs rely on {Get,Set}Response protobuf message fields to communicate errors i.e.:

repeated DataEntryError errors = 2;
Error error = 3;

This means the caller of either Get or Set needs to check those two fields to decide if there is an error or not. It is easy to miss for someone who would want to implement Get or Set and could lead to hidden errors. The nice thing about them though is that they provide detailed errors (one per data entry).

Subscribe and GetServerInfo don't have such error fields in their respective *Response protobuf messages. Instead, errors if any will result in an exception being raised (e.g. grpc.aio.AioRpcError in python's case) and that already comes without further efforts with protoc-generated code.

In some cases though, Get and Set might still raise grpc.aio.AioRpcError e.g. because of network errors. This means the client needs to handle both grpc.aio.AioRpcError errors and response-message-based errors.

It is unclear (i.e. unspecified) under which circumstances, which of the error fields (global or per Data Entry) will actually be used. In fact, I have not yet experienced a situation where the global error was set .... I have seen that when I get a single non-existent Data Entry, then the request succeeds and the response.error.code is 404. However, I haven't (yet) tried getting multiple entries of which some do not exist. In that case, the response could contain the values for the existing entries and a 404 for each of the non-exiting ones, right? That, however, would also feel quite inconsistent with the single non-existent entry case (sigh) ...

One more fundamental issue I see with being able to e.g. update multiple Data Entries in one call, is that it is unclear (i.e. unspecified) what happens if a subset of the Data Entries could be updated, but the others could not, e.g. due to wrong data type being used. Would this result in no entries being updated at all?

It also feels a little odd that, with this approach, the invocation seemed to have succeeded, but then you still need to inspect the error fields. Which, BTW, is a PITA because it means iterating through the whole data structure in order to determine if any of the Data Entry level error fields is set.

So, FMPOV the most important thing would be to actually properly document the error behavior of the gRPC API, because currently, it is basically a guessing/trial-and-error game ...

Apart from that, using the (extended) error info in the Status object makes more sense to me than encapsulating the error info in the response message(s).

[erikbosch]

We are about to archive this repo soon. If you consider this issue as important please file a new issue at one of the new Kuksa repos at https://github.com/eclipse-kuksa