Error-hook and the downsides of exceptions in java

[aepfli]

Currently our error hook specification reads as follow:

The `error` hook **MUST** run when errors are encountered in the `before` stage, the `after` stage or during flag resolution. It accepts `hook context` (required), `exception` representing what went wrong (required), and `hook hints` (optional). It has no return value.

which translates in Java to:

void error(HookContext<T> ctx, Exception error, Map<String, Object> hints)

This is great, and an easy way to simple handle exceptions., but it comes with a downside. We are using exception especially for the case where flags not found, or disabled for flow control.

This adds unnecessary overhead, and might not be the best approach as Java exception add overhead, and in this case those are not exceptions per se. Eg. in flagd we already implemented an approach, where we are not throwing those exception but use them as POJO. Still it is not a good coding practice to instantiate a class dedicated for throwing without throwing it.

So I am not sure, if we need to adapt the spec here, or if we only need to adapt the java interface.

Java proposal

We could instead of using the exception, create an own ErrorDetails.

public class ErrorDetails  {
    OpenFeatureError error;
    FlagEvaluationDetails evaluationDetails
}

and changing the interface to:

void error(HookContext<T> ctx, ErrorDetails error, Map<String, Object> hints)

Proposal to change description of hooks

The `error` hook **MUST** run when errors are encountered in the `before` stage, the `after` stage or during flag resolution. It accepts `hook context` (required), `errorDetails` representing what went wrong (required, **MUST** contain an exception or an errorneous FlagEvaluationDetails ), and `hook hints` (optional). It has no return value.

Question

should we only do this for the java world, or does an overall improvement for the error hook in general might be a good idea. Eg. having an own object would allow us to be more flexible in the future anyways.

WDYT?

[toddbaert]

IMO this is making a "flagd problem" into an "everyone problem".

In most cases, when error hooks run, there is some "Exceptional" circumstance, and, in most cases, and actual exception already from some underlying system or library (a JSON parse error, an HTTP status exception, etc). I think the real problem is that flagd considers DISABLED flags as NOT_FOUND, which is making an unexceptional circumstance (this flag is off) into an exceptional one (THIS FLAG DOES NOT EXIST! 😨) and that's the real root of the issue.

There's a proposal for improvements to flagd here, and in the flagd project board as well.

[aepfli]

I would see this flagd issue as a symptom, but not the root cause. Using Exceptions for all this expected errornous cases, even for other providers is the Problem. We are utilizing Exceptions as POJOs, and imho our hacky solution of just instantiating them but not throwing is not helping in the process. Plain POJOs or objects are enough (in the JAVA world) and even add more flexibility for the future. The exception itself is just a nice to have for the error hook, but not essential. We should have a clear abstraction around this, and imho, an error in the openFeature world does not immediately has to be an Exception, now per API we are forcing users into this.

... and as you said ... in most cases ... but not in all of them. A "Flag not found" is now always considered an exception, and forces everyone to use an exception - but in the end, is it really an exceptional case, something you need to react and do something immediately? often this stems from missing configuration, and a lot of dev teams start with flags, without configuring them, as the code default is enough for the beginning.

[aepfli]

i added a draft pullrequest i do think code is always better to tell a story than text :)

[chrfwow]

I agree with @aepfli here, creating an exception object without throwing it is probably not the best way to go. This is also evident in the way we create the exception, using some none-standard way so that we do not also create the stack trace and don't introduce the associated performance cost.