open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
639 stars 34 forks source link

Should `after` hooks be allowed to mutate the value? #48

Closed justinabrahms closed 2 years ago

justinabrahms commented 2 years ago

Pro: you can do a json -> instance mapping in there, which would be rad. Con: subsequent hooks don't get the json value anymore, which may be very surprising to them.

moredip commented 2 years ago

I think another con would be it would make type signatures really fiddly, and/or we'd lose a lot of type safety.

beeme1mr commented 2 years ago

We could allow the after hook to mutate the value but require that the type signature itself doesn't change.

toddbaert commented 2 years ago

In addition to side-effects that don't impact the returned value at all (ending an OTel span, logging, etc), one of the use-cases we have for this is validation of a resolved flag value. However, that use-case is resolved simply by throwing in the after hook if the retrieved value is invalid, which results in the evaluation defaulting, which is probably exactly what we'd want.

That is to say, I think we can, for now, close this since both those basic use-cases are satisfied. I don't think we need to support mutation of the after value, for now. Please re-open if you disagree!