Allow plugins to return error information as query responses

[cstepanian]

• Add error field to Query schema in protobuf
• Add error: Option<String> to struct Query in hipcheck-common/src/types.rs
• Add error variant to QueryState enum in protobuf
• Add Error case to PluginResponse

Remaining TODOs

• Resolve CI build error: Error: protoc failed: hipcheck.proto: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set.
• Create PluginResponse::Error when appropriate in its impl From<Query> block
• Make sure that handle_session passes error responses to send_session_err
• Update send_session_err to send error strings instead of just setting QueryState to Unspecified
• Test that the changes are effective
  • Change a plugin's use of QueryState::Unspecified to QueryState::Error and pass an error string
  • Confirm that Hipcheck prints the error
• Ensure all possible data paths for plugin errors eventually get printed in a useful way
• Update documentation
• Write RFD that explains changes

[j-lanson]

I don't understand why this needs an additional state variant - "Unspecified" would only be used in an error scenario anyway, it's short for "UnspecifiedError". we can also re-name variants without breaking compatibility, so we could just rename "Unspecified" to "Error"

[cstepanian]

That's correct. I chose to add a new state in the draft version of the PR, mostly because it was easier to distinguish between the new uses (that include an error string) and the old ones (which don't).

Ideally, I'd like to update all existing uses of Unspecified to use error strings. The idea would be to incrementally replace all of them with Error, and finally rename Unspecified to Error when all are converted.

But I wasn't sure if that would end up being too many changes for one PR (both in terms of dev time, and review effort).

[j-lanson]

Is there a reason we can't re-use an existing field like "key" or "concern" for the error message? Not only does it preserve the existing structure, but we can guarantee "error" can only be treated as an error message when the state matches

[cstepanian]

It should certainly be possible to re-use an existing field. For my sanity while prototyping the changes, I added a new field to keep it separate.