perf: improve error handling

[thisthat]

This PR

While working with OF, I noticed that in case a flag is not found, the whole stack trace is attached to the exception.
I think this info is not necessary for this type of exception since it is hidden from the end user, and the SDK only requires the ErrorCode field.

I quickly configured some JMH tests that use the InMemory FlagD provider to test the performance gains in evaluating a flag that doesn't exist. The Score value represents the amount of FF evaluations done in a single millisecond.
In a single thread case, the performance gain is >60%.

Before

"Benchmark","Mode","Threads","Samples","Score","Score Error (99,9%)","Unit"
"booleanNotFound_01Thread","thrpt",1,10,"459,324808","8,254488","ops/ms"
"booleanNotFound_02Thread","thrpt",2,10,"791,244898","8,644711","ops/ms"
"booleanNotFound_05Thread","thrpt",5,10,"1465,736432","17,059916","ops/ms"

After

"Benchmark","Mode","Threads","Samples","Score","Score Error (99,9%)","Unit"
"booleanNotFound_01Thread","thrpt",1,10,"765,474969","17,161956","ops/ms"
"booleanNotFound_02Thread","thrpt",2,10,"1159,854112","23,483354","ops/ms"
"booleanNotFound_05Thread","thrpt",5,10,"1828,460599","45,932954","ops/ms"

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (99bd988) 94.86% compared to head (55c303e) 95.10%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #739      +/-   ##
============================================
+ Coverage     94.86%   95.10%   +0.23%     
- Complexity      365      367       +2     
============================================
  Files            33       34       +1     
  Lines           857      858       +1     
  Branches         53       53              
============================================
+ Hits            813      816       +3     
+ Misses           23       22       -1     
+ Partials         21       20       -1     

Flag	Coverage Δ
unittests	95.10% <100.00%> (+0.23%)	⬆️

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.

[beeme1mr]

Do we need the stack trace on any OpenFeatureError?

[toddbaert]

Agree with this. Just see here.

[toddbaert]

Do we need the stack trace on any OpenFeatureError?

We have cases where consumers do things like:

catch (IOException e) {
  throw new GeneralError("Something bad happend", e);
}

Here, e is the "cause" exception. This is especially important for things like GeneralError where something unexpected has happened due to a logic error or system failure. I don't know how fillInStackTrace impacts this. I presume the "Caused by" normally seen in the stack-trace would be lost, and therefore I think in logs, etc, the underlying "cause" would be lost as well.

@thisthat @beeme1mr

[thisthat]

@toddbaert the suggested fillInStackTrace() implementation prints only dev.openfeature.sdk.exceptions.FlagNotFoundError instead of the full list of methods calls from the main to the throw of the exception.
For end users, the stack trace is not relevant because they (hopefully) would never see any exception.
So I think we should look into the code of existing providers and how they use the exceptions.

[toddbaert]

@toddbaert the suggested fillInStackTrace() implementation prints only dev.openfeature.sdk.exceptions.FlagNotFoundError instead of the full list of methods calls from the main to the throw of the exception. For end users, the stack trace is not relevant because they (hopefully) would never see any exception. So I think we should look into the code of existing providers and how they use the exceptions.

@thisthat Yes, I understand that. I was making a point relative to @beeme1mr 's question:

Do we need the stack trace on any OpenFeatureError?

My concern is that if we do what you've done here for other exceptions which are more complicated than FlagNotFound, we lose the causal information (I think). See this example: https://github.com/open-feature/java-sdk-contrib/blob/a315b7bc351a432fb96dfa9223180652c40d8736/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java#L371

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud