Skip to content

spec: Add UNEXPECTED_FIELD general error code. #109

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

Conversation

gpaul
Copy link

@gpaul gpaul commented Sep 20, 2017

This PR adds a general error code that can be used when optional fields are
provided when they are not expected.

These cases are:

  • NodePublishVolumeRequest.publish_volume_info is provided
    even though the corresponding Controller Plugin has no
    PUBLISH_UNPUBLISH_VOLUME capability.
  • ControllerPublishVolumeRequest.node_id is specified
    even though none was returned from GetNodeID.
  • ControllerUnpublishVolumeRequest.node_id is specified
    even though none was returned from GetNodeID.

This PR adds a general error code that can be used when optional fields
are provided when they are not expected.

These cases are:

- NodePublishVolumeRequest.publish_volume_info is provided even though
  the corresponding Controller Plugin has no `PUBLISH_UNPUBLISH_VOLUME`
  capability.
- ControllerPublishVolumeRequest.node_id is specified even though none
  was returned from GetNodeID.
- ControllerUnpublishVolumeRequest.node_id is specified even though
  none was returned from GetNodeID.
@gpaul
Copy link
Author

gpaul commented Sep 20, 2017

The alternative is to use GeneralError: UNDEFINED with a suitable ErrorDescription. Then again, that's true for virtually every error that reports incorrect plugin<->CO semantics, hence this PR.

@cpuguy83
Copy link
Contributor

cpuguy83 commented Oct 3, 2017

In general I feel like we should be moving to fewer codes.
I generally see these codes as a class of error, e.g. bad user input and then a descriptive message.
For a "class of error", I see this as "how will the caller react to this", and in the case of bad user input it will always be to tell the user they did something wrong.

There is a PR open from @saad-ali to move error handling over to use GRPC error codes rather than custom error codes: #115 which I think is relevant here.

@jdef
Copy link
Member

jdef commented Nov 14, 2017

this should probably be re-cast now that #115 has landed. maybe as a case where InvalidArgument may be returned?

@jdef
Copy link
Member

jdef commented Feb 26, 2018

The spec documents that InvalidArgument should be returned for this case, for all RPCs. Closing this out. See https://github.com/container-storage-interface/spec/blob/master/spec.md#error-scheme

@jdef jdef closed this Feb 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants