Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add enumerated failure reasons to gRPC response structures #254

Closed
scottbrewer opened this issue Oct 4, 2023 · 3 comments
Closed

Add enumerated failure reasons to gRPC response structures #254

scottbrewer opened this issue Oct 4, 2023 · 3 comments
Assignees
Labels

Comments

@scottbrewer
Copy link

Hi team -

This is an enhancement request to the gRPC proto that would return an enumerated detailed failure reason on gRPC requests that cannot be completed successfully. Today a boolean status field is all that exists in most response types, but a failed/false status does not give the client/user any indication of what went wrong. It would be nice to be able to determine the difference between a create request that failed because an input parameter was malformed versus because the resource already exists (can be treated as success in some cases).

I recommend the following as a minimum:

enum StatusReason {
    STATUS_REASON_NO_ERROR = 0
    STATUS_REASON_BAD_REQUEST = 1
    STATUS_REASON_RESOURCE_EXISTS = 2
    STATUS_REASON_RESOURCE_NOT_FOUND = 3
    STATUS_REASON_RESOURCE_BUSY = 4
    STATUS_REASON_INTERNAL_ERROR = 5
}

Once the enum list is created, all response types should be updated to include a status and status reason (most have status already) and the server can be updated to set the fields appropriately in error paths.

Thanks

@github-project-automation github-project-automation bot moved this to 🆕 New in NVMe-oF Oct 4, 2023
@caroav caroav added the CLI label Nov 9, 2023
@caroav
Copy link
Collaborator

caroav commented Nov 14, 2023

@oritwas please add your input here.

@oritwas
Copy link
Member

oritwas commented Nov 14, 2023

We should use existing standard status codes and not invent our own. We can use the grpc error codes,
https://grpc.github.io/grpc/core/md_doc_statuscodes.html or the standard Linux error codes

@caroav caroav added gRPC and removed CLI labels Nov 19, 2023
@gbregman
Copy link
Contributor

gbregman commented Jan 7, 2024

This was implemented in PRs #343 and #352. The status returned by gRPC is now an errno code. Notice, that in most cases we don't know the exact problem as it is internal to SPDK. We just get a boolean status from SPDK whether the operation was successful or not.

@gbregman gbregman closed this as completed Jan 7, 2024
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NVMe-oF Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

4 participants