Skip to content

Conversation

aaron-steinfeld
Copy link
Contributor

Description

This interceptor is meant to be used in edge services to prevent leaking any sensitive information. It only propagates status code, an external message and request ID, if set. All other metadata such as cause and metadata are stripped.

@aaron-steinfeld aaron-steinfeld requested a review from sarthak77 May 31, 2024 01:53
@aaron-steinfeld aaron-steinfeld requested a review from a team as a code owner May 31, 2024 01:53
Copy link

github-actions bot commented May 31, 2024

Test Results

82 tests  ±0   82 ✅ ±0   22s ⏱️ +4s
14 suites ±0    0 💤 ±0 
14 files   ±0    0 ❌ ±0 

Results for commit d2a6b22. ± Comparison against base commit 1c57c76.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented May 31, 2024

Codecov Report

Attention: Patch coverage is 0% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 72.32%. Comparing base (1c57c76) to head (d2a6b22).

Files Patch % Lines
...grpcutils/server/ExternalExceptionInterceptor.java 0.00% 36 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #61      +/-   ##
============================================
- Coverage     77.66%   72.32%   -5.34%     
  Complexity      172      172              
============================================
  Files            24       25       +1     
  Lines           488      524      +36     
  Branches         26       26              
============================================
  Hits            379      379              
- Misses           85      121      +36     
  Partials         24       24              
Flag Coverage Δ
unit 72.32% <0.00%> (-5.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sarthak77
sarthak77 previously approved these changes May 31, 2024
* This interceptor can be used at the edge to scrub any sensitive information such as an exception
* cause or metadata off the context before propagating it
*/
public class ExternalExceptionInterceptor implements ServerInterceptor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we define this as an abstract class so that one can define custom message and custom trailer building logic 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an abstract class and we typically would not expect these t o be configured differently in different services but I'll open up a couple of protected hooks for extension if needed.

@aaron-steinfeld aaron-steinfeld merged commit 9051c12 into main May 31, 2024
@aaron-steinfeld aaron-steinfeld deleted the add-external-error-interceptor branch May 31, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants