Skip to content

Conversation

aaron-steinfeld
Copy link
Contributor

Description

Made context propagation automatic, removed requirement to provide context when creating context.

@aaron-steinfeld aaron-steinfeld requested a review from a team as a code owner December 29, 2023 16:10
Copy link

codecov bot commented Dec 29, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (d69cbdf) 75.50% compared to head (15ff851) 79.28%.

Files Patch % Lines
...tils/context/ContextualStatusExceptionBuilder.java 76.47% 4 Missing ⚠️
.../grpcutils/context/ContextualExceptionDetails.java 75.00% 1 Missing and 1 partial ⚠️
...grpcutils/server/ThrowableResponseInterceptor.java 88.88% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #56      +/-   ##
============================================
+ Coverage     75.50%   79.28%   +3.78%     
- Complexity      162      172      +10     
============================================
  Files            23       23              
  Lines           449      478      +29     
  Branches         24       25       +1     
============================================
+ Hits            339      379      +40     
+ Misses           88       75      -13     
- Partials         22       24       +2     
Flag Coverage Δ
unit 79.28% <81.39%> (+3.78%) ⬆️

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.

Copy link

Test Results

82 tests  +6   82 ✅ +6   23s ⏱️ +10s
14 suites +1    0 💤 ±0 
14 files   +1    0 ❌ ±0 

Results for commit 15ff851. ± Comparison against base commit d69cbdf.

}
// Otherwise, we've found a status so collect any trailers from it and merge them on top of
// any trailers we can find from descendents
Metadata trailersFromCauseDescendents = this.collectAllTrailersFromCause(statusFromCause);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this collection grow out of bounds affecting memory?

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 really a concern. It only recurses for the length of the causal chain, so it's bounded by the size of that. As soon as we run out of causes or hit an unknown cause, we exit recursion (the two base cases above). The causal chain must necessarily be the same depth as this recursion (or longer), and each link in that is a throwable that contains a superset of the data we're copying out.

@aaron-steinfeld aaron-steinfeld merged commit 1f94e38 into main Jan 3, 2024
@aaron-steinfeld aaron-steinfeld deleted the standard-exceptions-2 branch January 3, 2024 14:20
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.

2 participants