Provide more debugging info for RpcStatus

[rshearman]

• grpc-sys: provide error string for receive status on client calls
• call: provide debug error string in RpcStatus
• ~error: make Display ouput more human-readable~
• ~call: make Display of RpcStatusCode more human-readable~

Example of contents of RpcStatus::debug_error_string:

thread 'test_health_watch_multiple' panicked at 'called `Result::unwrap()` on an `Err` value: RpcFailure(RpcStatus { code: 14-UNAVAILABLE, message: "failed to connect to all addresses", details: [], debug_error_string: "{\"created\":\"@1672417943.726323715\",\"description\":\"Failed to pick subchannel\",\"file\":\"/home/rob/grpc-rs/grpc-sys/grpc/src/core/ext/filters/client_channel/client_channel.cc\",\"file_line\":3217,\"referenced_errors\":[{\"created\":\"@1672417943.726320053\",\"description\":\"failed to connect to all addresses\",\"file\":\"/home/rob/grpc-rs/grpc-sys/grpc/src/core/lib/transport/error_utils.cc\",\"file_line\":165,\"grpc_status\":14}]}" })', health/tests/health_check.rs:137:5

[rshearman]

The debug_string part LGTM, the rest seems unnecessary.

Could I ask why? As an example, currently, we are displaying an error to a user and the Display trait for Error outputs it as:

RpcFailure: 7-PERMISSION_DENIED Unknown or deactivated device

IMHO it would be nicer and easier to understand for the user to output it as:

RPC failure: Unknown or deactivated device

Which seems to better match the intent of the Display trait being intended for "user-facing output" (https://doc.rust-lang.org/std/fmt/trait.Display.html).

[BusyJay]

it would be nicer and easier to understand for the user to output it as:

I suppose the user here means developers. For developers, error code is an essential part of the RPC response, code is well defined, the message may change because of implementations, but code won't. For example, seeing 404 in HTTP Response, most developers will notice the path may not exist immediately without looking into the messages. Display and Debug don't have much difference when targeting developers in the RPC domain.

If you want to display a "better" message to normal users like users that use your application, then you may consider change the Display implementation of error type defined in your application instead of leverage the implementation to the library directly. As "better" is a relative concept in different context. One format may seem better in your case while not in others.

[rshearman]

I suppose the user here means developers.

The user in my case really is a user, not a developer.

If you want to display a "better" message to normal users like users that use your application, then you may consider change the Display implementation of error type defined in your application instead of leverage the implementation to the library directly. As "better" is a relative concept in different context. One format may seem better in your case while not in others.

I can absolutely do that, but my motivation here was to give others the benefit of the change. I understand there is a tradeoff, but I was hoping in making a bigger distinction between the Debug and Display trait implementations that applications can choose one or the other depending on their needs.

If I haven't been able to convince you that the Display implementation changes are worthwhile, I'll remove them from this PR - just let me know.

[BusyJay]

The target users of the library are developers, so we better keep the original format. :)

[rshearman]

I've removed the changes to the Display trait implementations.

[rshearman]

Various CI jobs are failing, but these appear to match a scheduled run without my changes, e.g.: https://github.com/tikv/grpc-rs/actions/runs/3803392868