[GRPC] Add separate grpc.detailed_errors.enabled error handling setting for gRPC #19644
Conversation
…setting for gRPC Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
github-actions
bot
added
enhancement
Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
|
❌ Gradle check result for 702f7d9: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19644 +/- ##
============================================
+ Coverage 73.12% 73.13% +0.01%
- Complexity 70973 70979 +6
============================================
Files 5737 5737
Lines 324767 324769 +2
Branches 46982 46980 -2
============================================
+ Hits 237483 237528 +45
- Misses 68151 68168 +17
+ Partials 19133 19073 -60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
|
❌ Gradle check result for 9b42790: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Looks like a failure due to a flaky test, more here: #14312 |
|
Hi folks, this PR is ready for review, could someone please take a look? Thanks a lot! |
| * @param globalRequestParams The global parameters from the gRPC request | ||
| * @throws IllegalArgumentException if error tracing is requested but disabled by the server side | ||
| */ | ||
| public static void validateErrorTracingConfiguration(boolean detailedErrorsEnabled, GlobalParams globalRequestParams) { |
There was a problem hiding this comment.
Where in this PR does it handle truncation of the error to return a shortened error message if detailedErrorsEnabled is false?
There was a problem hiding this comment.
Thanks for the review @karenyrx! Sorry, I should have mentioned such details in the PR description.
The scope of this PR was just to add the server side setting and the incompatible configuration validation (i.e. the server side detailed tracing is off but a client explicitly requests details). Otherwise error details handling stays as it is done right now.
I have changes lined up for adding error summary and/or detailed stacktraces here. I'd need to clean it up and add testing but the changes for truncation will be coming soon in the scope of this issue #19092.
I was thinking, having 2 separate PRs would work better since there’s less code to review but, totally, I can rework this PR if needed.
The CI can be rerun by closing/re-opening the PR, or pushing an empty commit. (Or by resolving conflicts which creates a merge commit) |
Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
…ror-handling-setting
Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
2 participants
Description
This PR introduces a new setting
grpc.detailed_errors.enabled(much like the similar http flag.)This way gRPC APIs would have the same semantics when using the global
error_tracerequest parameter once it is implemented.Related Issues
Resolves #19639
Check List
[ ] Public documentation issue/PR created, if applicable.Please note, that I haven't yet created the public documentation issue/PR for this setting as first I wanted to give this PR a go, get more feedback and then create the corresponding issue/PR.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.