feat: error resolution flow control without exceptions

[aepfli]

So far, we used exceptions to handle our flow control, but Exceptions are costly. In the end, we enriched our
evaluation Details with errorCode and errorMessage. If desired, the providers can also handle this to reduce the execution footprint in erroneous cases, which don't have to be exceptions. Till now, we also converted those into exceptions and threw them, with this little code change, we would return already the details, and call the hooks, without exceptions being thrown.

For example, FlagNotFound is such a case; it can be expected and not exceptional. Hence, the exception flow might add too much overhead to performance-critical environments.

This implementation now allows the prover implementors to decide whether to support either or both.

Follow-up Tasks

Most likely this is something we also want to do in all other sdks.

How to test

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.34%. Comparing base (29901b8) to head (430ba1c).
Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1095      +/-   ##
============================================
+ Coverage     95.09%   95.34%   +0.25%     
- Complexity      400      402       +2     
============================================
  Files            39       39              
  Lines           917      924       +7     
  Branches         56       56              
============================================
+ Hits            872      881       +9     
+ Misses           24       23       -1     
+ Partials         21       20       -1     

Flag	Coverage Δ
unittests	95.34% <100.00%> (+0.25%)	⬆️

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.

[aepfli]

I am not good with performance tests, but I tried the following locally:

        IntStream.range( 0, 1000000 ).parallel().forEach(i -> {
            String key = "key" + i;
            c.getBooleanDetails(key, false);
        });

With the exceptions, it took more than 3s; with details, it took only 1.5s.

Remarks:

• I changed the implementation of the AlwaysBrokenWithDetailsProvider to set a different message all the time so that we never create the same object if the JIT somehow starts to optimize.
• I removed the asserts because they took most of the time, and i was only aiming for run speed.
• I ran those tests for the fun about 5 times, with close results.
• I know this is not the best performance test, but it gives a little bit of insights

// EDIT: forget about my tests, i might have made a mistake - i cant reproduce this - hence that, those are bad tests.

// EDIT: https://www.baeldung.com/java-exceptions-performance is a nice comprehensive post about this

[toddbaert]

I agree with @guidobrei 's point here.

In addition, I'm wondering if we should remove stack traces from some or all Errors, in addition to the FlagNotFound.

I think for a few, like ProviderNotReady and TypeMismatch we can also remove the stack trace, especially since these will be frequently instantiated by the SDK here and it's very obvious what the problem is.

[aepfli]

In addition, I'm wondering if we should remove stack traces from some or all Errors, in addition to the FlagNotFound.

I think for a few, like ProviderNotReady and TypeMismatch we can also remove the stack trace, especially since these will be frequently instantiated by the SDK here and it's very obvious what the problem is.

I added this via a parent class to the errors mentioned. This way, we can easily identify them later. Wdyt? (we can also try to achieve this with an interface and default methods)

[guidobrei]

I think for a few, like ProviderNotReady and TypeMismatch we can also remove the stack trace, especially since these will be frequently instantiated by the SDK here and it's very obvious what the problem is.

Not sure for the TypeMismatch. If client.getBooleanValue("flag-key", false); is called from user code for a string flag, I'd like to see the root cause.

[toddbaert]

I think for a few, like ProviderNotReady and TypeMismatch we can also remove the stack trace, especially since these will be frequently instantiated by the SDK here and it's very obvious what the problem is.

Not sure for the TypeMismatch. If client.getBooleanValue("flag-key", false); is called from user code for a string flag, I'd like to see the root cause.

OK I'm fine with that.

[toddbaert]

Approved but you can make the TypeMismatch change @guidobrei suggestes.

[jarebudev]

I am not good with performance tests, but I tried the following locally:

        IntStream.range( 0, 1000000 ).parallel().forEach(i -> {
            String key = "key" + i;
            c.getBooleanDetails(key, false);
        });

With the exceptions, it took more than 3s; with details, it took only 1.5s.

Remarks:

• I changed the implementation of the AlwaysBrokenWithDetailsProvider to set a different message all the time so that we never create the same object if the JIT somehow starts to optimize.
• I removed the asserts because they took most of the time, and i was only aiming for run speed.
• I ran those tests for the fun about 5 times, with close results.
• I know this is not the best performance test, but it gives a little bit of insights

// EDIT: forget about my tests, i might have made a mistake - i cant reproduce this - hence that, those are bad tests.

// EDIT: https://www.baeldung.com/java-exceptions-performance is a nice comprehensive post about this

This has got me wondering if there is value in creating some JMH benchmarks for the sdk...?

[sonarcloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud