launchdarkly / ios-client-sdk

LaunchDarkly Client-side SDK for iOS (Swift and Obj-C)
https://docs.launchdarkly.com/sdk/client-side/ios
Other
68 stars 84 forks source link

LDValue only supports Double numerics #272

Closed eseay closed 2 years ago

eseay commented 2 years ago

Describe the bug Int values in JSON variations are resolved as Double in the LaunchDarkly SDK.

This "bug" appears to be intentional in its design; however, I believe it leaves an important gap in functionality. If I have a JSON variation defined in the LD console that includes an Int value, then the current design requires me to always first read that value as a Double before casting it to an Int.

The issue presents itself when I am trying to map LDValue JSON objects into Decodable structs. Because the LDValue that is resolved by LaunchDarkly represents all numbers as Double, attempting to encode the LDValue to Data and subsequently decode that data into a struct with Int properties will sometimes result in type mismatch failures.

To reproduce I can't reproduce this in a deterministic manner. What is happening though is that in the decoding/encoding process, the value that is actually an Int is losing precision since it's being treated as a Double in LaunchDarkly. I can see this issue occurring tens of thousands of times in my bug reporting tools.

Expected behavior I would expect the LDValue type to include both an .int and .double (or .float) case instead of just a .number case. Because JSON data is represented as a String, Int values can be treated as precise, so the current implementation of LDValue leaves a functionality gap.

Logs N/A

SDK version 6.1.0

Language version, developer tools Swift 5.6, Xcode 13.4.1

OS/platform All versions of iOS that my app runs on

louis-launchdarkly commented 2 years ago

For the record, this request is currently going through LaunchDarkly Support as we need to clarify more from @eseay.

eseay commented 2 years ago

@louis-launchdarkly Yes, thank you! I don't want to create too much dialogue in separate threads, but GitHub makes it way easier to put a code sample than an email thread, so I'll share a little sample here to clarify.

I do realize now that my "expected behavior" is a bit prescriptive for a solution, so to restate the above a bit more elegantly, what I am seeing is:

  1. Start with a JSON variation in LaunchDarkly which includes a key with a whole, non-decimal Int value;

    {
    "stringKey": "foo",
    "intKey": 2
    }
  2. Read data from (1) into my app as an LDValue using the SDK;

    
    let val = client.jsonVariation(forKey: "some-flag-key", defaultValue: [:])

// val == the following LDValue.object([ "stringKey": LDValue.string("foo"), "intKey": LDValue.number(2.0)
])


3. Use `JSONEncoder` to encode the `LDValue` from (2) back into JSON data;
```swift
let jsonData = try JSONEncoder().encode(val)
  1. Use JSONDecoder to decode JSON data from (3) into a Codable Swift struct with a schema matching that of the data in (1). Decoding intermittently fails with a type mismatch.
    
    struct MyVariation: Codable {
    let stringKey: String
    let intKey: Int
    }

do { let decoded = try JSONDecoder().decode(MyVariation.self, from: jsonData) } catch { // DecodingError thrown intermittently with the following description details. // Note that I have it as an NSError because the error is getting converted to // that when I send it to my error reporter. // // code=4864 // domain=NSCocoaErrorDomain // reason=The data isn't in the correct format // userInfo={ // NSDebugDescription="Expected to decode Int but found a number instead." // NSCodingPath=CodingKeys(stringValue: "intKey", intValue: nil) // } }


This is getting done by means of a protocol + extension to which I conform these structs. It's purpose is to save a mountain of boilerplate and handwritten code:

```swift
protocol LDValueInitializable {
    init(ldValue: LDValue) throws
}

extension LDValueInitializable where Self: Decodable {
    init(ldValue: LDValue) throws {
        // An error that gets eaten downstream that is thrown when we hit a default value.
        // I highlight this to emphasize that the default value case is _not_ the origin of
        // the issue we're seeing        
        guard !ldValue.isEmptyDictionary else { throw AdhocError(reason: "empty_dictionary") }

        do {
            let data = try JSONEncoder().encode(ldValue)
            self = try JSONDecoder().decode(Self.self, from: data)
        } catch {
            // send error data to error tracker
            throw error
        }
    }
}

I'll also add that I am using this same approach for 3 other types in a total of 6 other places. None of the other models include integer numeric values, so this is the only place the issue is surfacing.

Let me know if there is any more detail I can add. Thanks!

eseay commented 2 years ago

@louis-launchdarkly Just wanted to close the loop here -

The workaround we currently have deployed for this is a Codable property wrapper that we're annotating the property in the model with. This ensures that no matter how the JSONEncoder re-encodes the LDValue to JSON, that the property will be treated as we want by the following JSONDecoder.

I attempted to enhance this property wrapper to accept a parameter defining the FloatingPointRoundingRule, but applying and storing that state on the property wrapper proved problematic. Thanks!

@propertyWrapper struct DecodeDoubleAsInt: Codable, Hashable {
    public var wrappedValue: Int

    public init(wrappedValue: Int) {
        self.wrappedValue = wrappedValue
    }

    public init(from decoder: Decoder) throws {
        let container = try decoder.singleValueContainer()
        do {
            // attempt to decode the value as an Int
            self.wrappedValue = try container.decode(Int.self)
        } catch {
            // if decoding as Int fails, try to decode as Double;
            // if that fails, throw the error as expected
            let doubleValue = try container.decode(Double.self)
            // round the double using default rounding behavior
            self.wrappedValue = Int(doubleValue.rounded())
        }
    }

    public func encode(to encoder: Encoder) throws {
        try wrappedValue.encode(to: encoder)
    }
}

/// Example usage
struct MyExample: Codable {
    let stringValue: String
    @DecodeDoubleAsInt var intValue: Int   
}

cc: @professorice