From 635e31308557a3e0b7f31058a40a0b5546ab0d64 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Thu, 20 Feb 2025 17:22:27 +0100 Subject: [PATCH] fix: [OF] Better error reporting in some cases Signed-off-by: Fabrizio Demaria --- Sources/Confidence/ConfidenceError.swift | 22 +++++++++ Sources/Confidence/FlagEvaluation.swift | 14 +++++- .../ConfidenceFeatureProvider.swift | 5 ++ .../ConfidenceProviderTest.swift | 48 +++++++++++++++++++ Tests/ConfidenceTests/ConfidenceTest.swift | 43 +++++++++++++++++ 5 files changed, 130 insertions(+), 2 deletions(-) diff --git a/Sources/Confidence/ConfidenceError.swift b/Sources/Confidence/ConfidenceError.swift index 031c21ce..847a143f 100644 --- a/Sources/Confidence/ConfidenceError.swift +++ b/Sources/Confidence/ConfidenceError.swift @@ -9,6 +9,28 @@ public enum ConfidenceError: Error, Equatable { case internalError(message: String) case parseError(message: String) case invalidContextInMessage + + var errorCode: ErrorCode { + switch self { + case .grpcError(let message), + .cacheError(let message), + .corruptedCache(let message), + .badRequest(let message?), + .internalError(let message): + return .generalError(message: message) + + case .flagNotFoundError: + return .flagNotFound + + case .parseError(let message): + return .parseError(message: message) + + case .invalidContextInMessage: + return .invalidContext + case .badRequest(message: .none): + return .generalError(message: "unknown error") + } + } } extension ConfidenceError: CustomStringConvertible { diff --git a/Sources/Confidence/FlagEvaluation.swift b/Sources/Confidence/FlagEvaluation.swift index 393dd8e1..afef8685 100644 --- a/Sources/Confidence/FlagEvaluation.swift +++ b/Sources/Confidence/FlagEvaluation.swift @@ -8,12 +8,14 @@ public struct Evaluation { public let errorMessage: String? } -public enum ErrorCode { +public enum ErrorCode: Equatable { case providerNotReady case invalidContext case flagNotFound case evaluationError + case parseError(message: String) case typeMismatch + case generalError(message: String) } struct FlagResolution: Encodable, Decodable, Equatable { @@ -130,6 +132,14 @@ extension FlagResolution { errorMessage: nil ) } + } catch let error as ConfidenceError { + return Evaluation( + value: defaultValue, + variant: nil, + reason: .error, + errorCode: error.errorCode, + errorMessage: error.description + ) } catch { return Evaluation( value: defaultValue, @@ -206,7 +216,7 @@ extension FlagResolution { private func getValue(path: [String], value: ConfidenceValue) throws -> ConfidenceValue { if path.isEmpty { - guard value.asStructure() != nil else { + guard value.asStructure() == nil else { throw ConfidenceError.parseError( message: "Flag path must contain path to the field for non-object values") } diff --git a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift index d88f1d10..eefa5044 100644 --- a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift +++ b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift @@ -99,6 +99,7 @@ public class ConfidenceFeatureProvider: FeatureProvider { public func getObjectEvaluation(key: String, defaultValue: OpenFeature.Value, context: EvaluationContext?) throws -> OpenFeature.ProviderEvaluation { + // Convert Struct Value to try confidence.getEvaluation(key: key, defaultValue: defaultValue).toProviderEvaluation() } @@ -130,6 +131,10 @@ extension Evaluation { throw OpenFeatureError.generalError(message: self.errorMessage ?? "unknown error") case .typeMismatch: throw OpenFeatureError.typeMismatchError + case .parseError(message: let message): + throw OpenFeatureError.parseError(message: message) + case .generalError(message: let message): + throw OpenFeatureError.generalError(message: message) } } return ProviderEvaluation( diff --git a/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift b/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift index 4d37f700..5855b2b4 100644 --- a/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift +++ b/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift @@ -129,6 +129,54 @@ class ConfidenceProviderTest: XCTestCase { } } } + + func testProviderThrowsParseErrorOnObjectEval() async throws { + let context = MutableContext(targetingKey: "t") + let readyExpectation = XCTestExpectation(description: "Ready") + let storage = StorageMock() + class FakeClient: ConfidenceResolveClient { + var resolvedValues: [ResolvedValue] = [ + ResolvedValue( + variant: "variant1", + value: .init(structure: ["int": .init(integer: 42)]), + flag: "flagName", + resolveReason: .match, + shouldApply: true) + ] + func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult { + return .init(resolvedValues: resolvedValues, resolveToken: "token") + } + } + + let confidence = Confidence.Builder(clientSecret: "test") + .withContext(initialContext: ["targeting_key": .init(string: "t")]) + .withFlagResolverClient(flagResolver: FakeClient()) + .withStorage(storage: storage) + .build() + + let provider = ConfidenceFeatureProvider(confidence: confidence, initializationStrategy: .fetchAndActivate) + OpenFeatureAPI.shared.setProvider(provider: provider) + let cancellable = OpenFeatureAPI.shared.observe().sink { event in + if event == .ready { + readyExpectation.fulfill() + } else { + print(event.debugDescription) + } + } + await fulfillment(of: [readyExpectation], timeout: 1.0) + cancellable.cancel() + XCTAssertThrowsError( + try provider.getIntegerEvaluation(key: "flagName", defaultValue: -1, context: context)) + { error in + if let specificError = error as? OpenFeatureError { + XCTAssertEqual(specificError.errorCode(), ErrorCode.parseError) + XCTAssertEqual(specificError.description, "Parse error: Flag path " + + "must contain path to the field for non-object values") + } else { + XCTFail("expected a flag not found error") + } + } + } } private class StorageMock: Storage { diff --git a/Tests/ConfidenceTests/ConfidenceTest.swift b/Tests/ConfidenceTests/ConfidenceTest.swift index 235cd9b2..f8aafaa6 100644 --- a/Tests/ConfidenceTests/ConfidenceTest.swift +++ b/Tests/ConfidenceTests/ConfidenceTest.swift @@ -201,6 +201,49 @@ class ConfidenceTest: XCTestCase { XCTAssertEqual(flagApplier.applyCallCount, 1) } + func testResolveStructureFails() async throws { + class FakeClient: ConfidenceResolveClient { + var resolveStats: Int = 0 + var resolvedValues: [ResolvedValue] = [] + func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult { + self.resolveStats += 1 + return .init(resolvedValues: resolvedValues, resolveToken: "token") + } + } + + let client = FakeClient() + client.resolvedValues = [ + ResolvedValue( + variant: "control", + value: .init(structure: ["size": .init(integer: 3)]), + flag: "flag", + resolveReason: .match, + shouldApply: true) + ] + + let confidence = Confidence.Builder(clientSecret: "test") + .withContext(initialContext: ["targeting_key": .init(string: "user2")]) + .withFlagResolverClient(flagResolver: client) + .withFlagApplier(flagApplier: flagApplier) + .build() + + try await confidence.fetchAndActivate() + let evaluation = confidence.getEvaluation( + key: "flag", + defaultValue: ConfidenceValue.init(structure: [:])) + + + XCTAssertEqual(client.resolveStats, 1) + XCTAssertEqual(evaluation.value, ConfidenceValue.init(structure: [:])) + XCTAssertEqual(evaluation.errorCode, .parseError(message: + "Flag path must contain path to the field for non-object values")) + XCTAssertEqual(evaluation.errorMessage, "Parse error occurred: Flag path must " + + "contain path to the field for non-object values") + XCTAssertEqual(evaluation.reason, .error) + XCTAssertNil(evaluation.variant) + XCTAssertEqual(client.resolveStats, 1) + XCTAssertEqual(flagApplier.applyCallCount, 0) + } func testResolveIntegerFlagWithInt64() async throws { class FakeClient: ConfidenceResolveClient {