chore: update code to be compatible with protobuf v36 - #16342
Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades the Protobuf dependency to version 36.0-rc2 across Bazel configurations, Dockerfiles, and workspace files, and casts several Protobuf method calls to (void) to ignore their return values. The reviewer feedback correctly notes that casting any.UnpackTo(&result) to (void) in extract_long_running_result.cc silently swallows unpacking errors, violating the repository's 'Demand Explosive Correctness' principle, and suggests returning an error status if unpacking fails.
| (void)any.UnpackTo(&result); | ||
| return Status{}; |
There was a problem hiding this comment.
Silently ignoring the return value of UnpackTo by casting it to (void) swallows potential errors when unpacking the operation metadata. If unpacking fails, we should return an appropriate error status instead of proceeding with a success status, in accordance with the repository style guide's 'Demand Explosive Correctness' principle.
if (!any.UnpackTo(&result)) {
return Status(StatusCode::kInvalidArgument,
"Failed to unpack operation metadata in operation " + op->name(),
GCP_ERROR_INFO());
}
return Status{};References
- Never swallow errors or ignore Status types. Fail loudly and explicitly when appropriate. (link)
| (void)any.UnpackTo(&result); | ||
| return Status{}; |
There was a problem hiding this comment.
Silently ignoring the return value of UnpackTo by casting it to (void) swallows potential errors when unpacking the operation response. If unpacking fails, we should return an appropriate error status instead of proceeding with a success status, in accordance with the repository style guide's 'Demand Explosive Correctness' principle.
if (!any.UnpackTo(&result)) {
return Status(StatusCode::kInvalidArgument,
"Failed to unpack operation response in operation " + op->name(),
GCP_ERROR_INFO());
}
return Status{};References
- Never swallow errors or ignore Status types. Fail loudly and explicitly when appropriate. (link)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16342 +/- ##
==========================================
- Coverage 92.23% 92.23% -0.01%
==========================================
Files 2227 2227
Lines 209283 209275 -8
==========================================
- Hits 193026 193017 -9
- Misses 16257 16258 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
No description provided.