Issue 1072: Error codes in TopicManagementResponse are incomplete, causing loss of important debug information #1073
Conversation
…elpful to use the reason String that came in as an parameter than UNKNOWN_ERROR. UNKNOWN_ERROR could be used for null or empty Strings.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
JosRoseboom
changed the title
jonathanedey
left a comment
jonathanedey
left a comment
There was a problem hiding this comment.
Thanks @JosRoseboom for putting this together! These changes look good with one comment.
| if (reason == null || reason.trim().isEmpty()) { | ||
| this.reason = UNKNOWN_ERROR; | ||
| } else { | ||
| this.reason = ERROR_CODES.getOrDefault(reason, reason); |
There was a problem hiding this comment.
I agree that unknown-error isn't very helpful here and in the future we want to avoid needlessly abstracting information like this. This is a reasonable stop gap until we can address this fully in a breaking change.
For consistency with the current format of the error reasons, could you add some additional logic here to map them from UPPER_SNAKE_CASE to lower-kebab-case.
3 participants
This pr contains 2 commits that provide a solution for the 2 problems described in #1072