Serialization Exeptions with Eval Context?

[justinabrahms]

How should we handle serialization/deserializtion exceptions around adding structures to an evaluation context?

I'm thinking: cyclical json, un-serializable type provided, etc.

I'm thinking as a user, I wouldn't want it to silently not add my context. That leaves throwing an exception? Don't love it, tbh.

[toddbaert]

I think this is a great question, and I even encountered a circular JSON error when I added a object with a referential loop into the context.

I think this is actually a great reason why the context transformer concept is useful. It exposes a great place for a provider to solve or expose problems like this, and others. For example, some providers have restrictions on the characters allowed in the key strings in the context (the provider may replace . with _ for instance in this case). Also, some providers support nested objects, and some do not. Providers can therefor flatten, mutate, throw, etc, essentially do whatever they want in their context transformer.

More directly to your point, I think it will be impossible not to ever throw when the context contains certain invalid things. Like I said, I found some vendor SDKs already threw (and defaulted) when supplied with circular objects which they apparently tried to serialize. I think an error thrown in the context transformer should probably cause the evaluation default, and based on existing SDK behavior I've seen, that makes sense.

[toddbaert]

@justinabrahms - any thoughts on the above? Here's an example of a context-transformer in our LaunchDarkly provider in the playground (which now uses the official Node SDK):

cc @agentgonzo, @moredip @dabeeeenster @InTheCloudDan

[justinabrahms]

I think what I'm taking from your example is "sometimes, adding objects to the context might throw exceptions".

I think this can happen regardless of if you use a context transformer, because the exception should probably happen at context addition time and not flag evaluation time.

What if you try to add an unserializable object to the structure in java land? We should throw there (if not a complier error).

[toddbaert]

I think what I'm taking from your example is "sometimes, adding objects to the context might throw exceptions".

Yes, I think so. Like I said:

More directly to your point, I think it will be impossible not to ever throw when the context contains certain invalid things.

...

I think this can happen regardless of if you use a context transformer, because the exception should probably happen at context addition time and not flag evaluation time.

Agreed, it just gives it a nice consistent place to throw/transform when we have such issues.

What if you try to add an unserializable object to the structure in java land? We should throw there (if not a complier error).

Fully agree. Not sure what else could be done, unless a provider want's to omit such objects silently (probably not a great idea).

[justinabrahms]

From the meeting, there are three primary options.

1. Throw at the moment context is added
2. Throw during flag evaluation time, returning a default value
3. Don't throw and just silently don't add something to the context.

3 is the least safe idea and I think we should just wholesale exclude it.

Between 1 and 2, I think we want the choice which is the safest for our users. With that, we have to assume that the provided default is safe. So I think the right choice is for us to not throw during adding to the context. The big reason for that is we may be adding some sort of runtime data structures to the context which we weren't able to test with. I don't think we want to be responsible for users throwing 5xx's for having some malformed data.

[toddbaert]

Between 1 and 2, I think we want the choice which is the safest for our users. With that, we have to assume that the provided default is safe. So I think the right choice is for us to not throw during adding to the context.

Agreed... this is also consistent with the behavior I've seen in existing SDKs, at least in JS.

What do you think about the point brought up in the meeting that some of this might be good to be configurable? Do you think it's worth making 1 vs 2 behavior dependent on an option, with 2 being the default?

[justinabrahms]

I think it's reasonable to say "you must, by default, do #2. You may, if you so choose, provide a way for #1 to happen based on user configuration".

[toddbaert]

@justinabrahms I'm interested in what your thoughts are on this now that things have matured quite a bit.

Is this still an issue?

[justinabrahms]

@toddbaert I've actually dropped serialization from the java sdk. This is now a provider concern.

[toddbaert]

@toddbaert I've actually dropped serialization from the java sdk. This is now a provider concern.

Makes sense. Closing.