Skip to content

Conversation

@cwaldren-ld
Copy link
Contributor

@cwaldren-ld cwaldren-ld commented Feb 7, 2024

Background
When the client-side SDK is deserializing an evaluation result, it may encounter a null evaluation value for a particular flag. That is, the value is literally null in JSON.

It might also receive an evaliuation result with an omitted value. Some server-side SDKs may produce this because they don't treat null/missing as equivalent.

Problem
In that case, the client-side SDK would throw a schema error. This isn't correct since producing a null/omitted value is legitimate.

Solution
The deserialization code now takes into account the equivalence of null/omission and doesn't throw an error.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #203949: Treat omitted JSON values as if they were null.

@cwaldren-ld cwaldren-ld changed the title fix(client-sdk): handle omitted evaluation result value when deseriazing fix(client-sdk): handle omitted evaluation result value when deserializing client-side JSON payload Feb 8, 2024
@cwaldren-ld cwaldren-ld changed the title fix(client-sdk): handle omitted evaluation result value when deserializing client-side JSON payload fix: handle omitted evaluation result value when deserializing client-side JSON payload Feb 12, 2024
@cwaldren-ld cwaldren-ld added the package: sdk/client Issues affecting the C++ Client SDK label Feb 12, 2024
@cwaldren-ld cwaldren-ld marked this pull request as ready for review February 13, 2024 22:17
@cwaldren-ld cwaldren-ld requested a review from a team February 13, 2024 22:17
@cwaldren-ld
Copy link
Contributor Author

Did a more deep audit of the code. I'm seeing an (un-contract-tested) problem in the handling of variations where null evaluation result will result in entering the type-check branch and return a WRONG_TYPE error.

That's not correct. So this PR shouldn't be merged yet until I make some contract tests that cover this (and fix it.)

@cwaldren-ld cwaldren-ld force-pushed the cw/sc-203949/handle-omitted-evaluation-result-value branch from e949229 to bb2b99f Compare March 12, 2024 19:57
@cwaldren-ld
Copy link
Contributor Author

I'm going to merge this as it's a good defensive change in case the SDK is sent bad/malformed data. I've created a ticket to investigate whether a null flag value is possible or not, and if we need to have contract tests to cover this.

@cwaldren-ld cwaldren-ld merged commit 334ea51 into main Mar 13, 2024
@cwaldren-ld cwaldren-ld deleted the cw/sc-203949/handle-omitted-evaluation-result-value branch March 13, 2024 19:48
This was referenced Mar 13, 2024
cwaldren-ld added a commit that referenced this pull request Mar 19, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>launchdarkly-cpp-client: 3.4.1</summary>

##
[3.4.1](launchdarkly-cpp-client-v3.4.0...launchdarkly-cpp-client-v3.4.1)
(2024-03-18)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-internal bumped from 0.5.4 to 0.6.0
</details>

<details><summary>launchdarkly-cpp-internal: 0.6.0</summary>

##
[0.6.0](launchdarkly-cpp-internal-v0.5.4...launchdarkly-cpp-internal-v0.6.0)
(2024-03-18)


### Features

* always inline contexts for feature events
([#362](#362))
([bc77e89](bc77e89))
* redact anonymous attributes within feature events
([#363](#363))
([85fe237](85fe237))


### Bug Fixes

* handle omitted evaluation result value when deserializing client-side
JSON payload
([#368](#368))
([334ea51](334ea51))
</details>

<details><summary>launchdarkly-cpp-server: 3.3.3</summary>

##
[3.3.3](launchdarkly-cpp-server-v3.3.2...launchdarkly-cpp-server-v3.3.3)
(2024-03-18)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-internal bumped from 0.5.4 to 0.6.0
</details>

<details><summary>launchdarkly-cpp-server-redis-source: 2.1.3</summary>

##
[2.1.3](launchdarkly-cpp-server-redis-source-v2.1.2...launchdarkly-cpp-server-redis-source-v2.1.3)
(2024-03-18)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-server bumped from 3.3.2 to 3.3.3
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Casey Waldren <cwaldren@launchdarkly.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: sdk/client Issues affecting the C++ Client SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants