Skip to content

Commit b99e53c

Browse files
toddbaerttoddbaert
committed
feat!: errorCode as enum, reason as string
Signed-off-by: Todd Baert <[email protected]>
1 parent e108666 commit b99e53c

17 files changed

+89
-44
lines changed

‎.github/workflows/pullrequest.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
${{ runner.os }}-maven-
3535
3636
- name: Build with Maven
37-
run: mvn --batch-mode --update-snapshots verify -P integration-test
37+
run: mvn --batch-mode --update-snapshots verify # -P integration-test - add this back once we have a compatible flagd
3838

3939
- name: Upload coverage to Codecov
4040
uses: codecov/codecov-action@v2

‎pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@
141141
<dependency>
142142
<groupId>dev.openfeature.contrib.providers</groupId>
143143
<artifactId>flagd</artifactId>
144+
<!-- TODO: update this version -->
144145
<version>0.3.2</version>
145146
<scope>test</scope>
146147
</dependency>

‎src/main/java/dev/openfeature/javasdk/BaseEvaluation.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ public interface BaseEvaluation<T> {
2121
* Describes how we came to the value that we're returning.
2222
* @return {Reason}
2323
*/
24-
Reason getReason();
24+
String getReason();
2525

2626
/**
2727
* The error code, if applicable. Should only be set when the Reason is ERROR.
2828
* @return {ErrorCode}
2929
*/
30-
String getErrorCode();
30+
ErrorCode getErrorCode();
3131
}

‎src/main/java/dev/openfeature/javasdk/ErrorCode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
package dev.openfeature.javasdk;
22

33
public enum ErrorCode {
4-
PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, GENERAL
4+
PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, TARGETING_KEY_MISSING, INVALID_CONTEXT, GENERAL
55
}

‎src/main/java/dev/openfeature/javasdk/FlagEvaluationDetails.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ public class FlagEvaluationDetails<T> implements BaseEvaluation<T> {
1414
private String flagKey;
1515
private T value;
1616
@Nullable private String variant;
17-
private Reason reason;
18-
@Nullable private String errorCode;
17+
@Nullable private String reason;
18+
private ErrorCode errorCode;
19+
@Nullable private String message;
1920

2021
/**
2122
* Generate detail payload from the provider response.

‎src/main/java/dev/openfeature/javasdk/NoOpProvider.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defa
2525
return ProviderEvaluation.<Boolean>builder()
2626
.value(defaultValue)
2727
.variant(PASSED_IN_DEFAULT)
28-
.reason(Reason.DEFAULT)
28+
.reason(Reason.DEFAULT.toString())
2929
.build();
3030
}
3131

@@ -34,7 +34,7 @@ public ProviderEvaluation<String> getStringEvaluation(String key, String default
3434
return ProviderEvaluation.<String>builder()
3535
.value(defaultValue)
3636
.variant(PASSED_IN_DEFAULT)
37-
.reason(Reason.DEFAULT)
37+
.reason(Reason.DEFAULT.toString())
3838
.build();
3939
}
4040

@@ -43,7 +43,7 @@ public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defa
4343
return ProviderEvaluation.<Integer>builder()
4444
.value(defaultValue)
4545
.variant(PASSED_IN_DEFAULT)
46-
.reason(Reason.DEFAULT)
46+
.reason(Reason.DEFAULT.toString())
4747
.build();
4848
}
4949

@@ -52,7 +52,7 @@ public ProviderEvaluation<Double> getDoubleEvaluation(String key, Double default
5252
return ProviderEvaluation.<Double>builder()
5353
.value(defaultValue)
5454
.variant(PASSED_IN_DEFAULT)
55-
.reason(Reason.DEFAULT)
55+
.reason(Reason.DEFAULT.toString())
5656
.build();
5757
}
5858

@@ -62,7 +62,7 @@ public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultVa
6262
return ProviderEvaluation.<Value>builder()
6363
.value(defaultValue)
6464
.variant(PASSED_IN_DEFAULT)
65-
.reason(Reason.DEFAULT)
65+
.reason(Reason.DEFAULT.toString())
6666
.build();
6767
}
6868
}

‎src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.util.Map;
88

99
import dev.openfeature.javasdk.exceptions.GeneralError;
10+
import dev.openfeature.javasdk.exceptions.OpenFeatureError;
1011
import dev.openfeature.javasdk.internal.ObjectUtils;
1112
import lombok.Getter;
1213
import lombok.Setter;
@@ -94,9 +95,14 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
9495
if (details == null) {
9596
details = FlagEvaluationDetails.<T>builder().build();
9697
}
98+
if (e instanceof OpenFeatureError) {
99+
details.setErrorCode(((OpenFeatureError)e).getErrorCode());
100+
} else {
101+
details.setErrorCode(ErrorCode.GENERAL);
102+
}
103+
details.setMessage(e.getMessage());
97104
details.setValue(defaultValue);
98-
details.setReason(Reason.ERROR);
99-
details.setErrorCode(e.getMessage());
105+
details.setReason(Reason.ERROR.toString());
100106
hookSupport.errorHooks(type, hookCtx, e, mergedHooks, hints);
101107
} finally {
102108
hookSupport.afterAllHooks(type, hookCtx, mergedHooks, hints);

‎src/main/java/dev/openfeature/javasdk/ProviderEvaluation.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
public class ProviderEvaluation<T> implements BaseEvaluation<T> {
1010
T value;
1111
@Nullable String variant;
12-
Reason reason;
13-
@Nullable String errorCode;
12+
@Nullable private String reason;
13+
ErrorCode errorCode;
14+
@Nullable private String message;
1415
}

‎src/main/java/dev/openfeature/javasdk/exceptions/FlagNotFoundError.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@
77
@StandardException
88
public class FlagNotFoundError extends OpenFeatureError {
99
private static final long serialVersionUID = 1L;
10-
@Getter private final ErrorCode errorCode = ErrorCode.GENERAL;
10+
@Getter private final ErrorCode errorCode = ErrorCode.FLAG_NOT_FOUND;
1111
}

‎src/main/java/dev/openfeature/javasdk/exceptions/InvalidContextError.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package dev.openfeature.javasdk.exceptions;
2+
3+
import dev.openfeature.javasdk.ErrorCode;
4+
import lombok.Getter;
5+
import lombok.experimental.StandardException;
6+
7+
@StandardException
8+
public class InvalidContextError extends OpenFeatureError {
9+
private static final long serialVersionUID = 1L;
10+
11+
@Getter private final ErrorCode errorCode = ErrorCode.INVALID_CONTEXT;
12+
13+
}

‎src/main/java/dev/openfeature/javasdk/exceptions/TargetingKeyMissingError.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package dev.openfeature.javasdk.exceptions;
2+
3+
import dev.openfeature.javasdk.ErrorCode;
4+
import lombok.Getter;
5+
import lombok.experimental.StandardException;
6+
7+
@StandardException
8+
public class TargetingKeyMissingError extends OpenFeatureError {
9+
private static final long serialVersionUID = 1L;
10+
11+
@Getter private final ErrorCode errorCode = ErrorCode.TARGETING_KEY_MISSING;
12+
13+
}

‎src/test/java/dev/openfeature/javasdk/AlwaysBrokenProvider.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,41 @@
11
package dev.openfeature.javasdk;
22

3+
import dev.openfeature.javasdk.exceptions.FlagNotFoundError;
34

45
public class AlwaysBrokenProvider implements FeatureProvider {
6+
57
@Override
68
public Metadata getMetadata() {
79
return new Metadata() {
810
@Override
911
public String getName() {
10-
throw new NotImplementedException("BORK");
12+
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
1113
}
1214
};
1315
}
1416

1517
@Override
1618
public ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) {
17-
throw new NotImplementedException("BORK");
19+
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
1820
}
1921

2022
@Override
2123
public ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) {
22-
throw new NotImplementedException("BORK");
24+
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
2325
}
2426

2527
@Override
2628
public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) {
27-
throw new NotImplementedException("BORK");
29+
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
2830
}
2931

3032
@Override
3133
public ProviderEvaluation<Double> getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) {
32-
throw new NotImplementedException("BORK");
34+
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
3335
}
3436

3537
@Override
3638
public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultValue, EvaluationContext invocationContext) {
37-
throw new NotImplementedException("BORK");
39+
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
3840
}
3941
}

‎src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ class DeveloperExperienceTest implements HookFixtures {
8585
api.setProvider(new AlwaysBrokenProvider());
8686
Client client = api.getClient();
8787
FlagEvaluationDetails<Boolean> retval = client.getBooleanDetails(flagKey, false);
88-
assertEquals("BORK", retval.getErrorCode());
89-
assertEquals(Reason.ERROR, retval.getReason());
88+
assertEquals(ErrorCode.FLAG_NOT_FOUND, retval.getErrorCode());
89+
assertEquals(TestConstants.BROKEN_MESSAGE, retval.getMessage());
90+
assertEquals(Reason.ERROR.toString(), retval.getReason());
9091
assertFalse(retval.getValue());
9192
}
9293
}

‎src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTest.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,16 @@ private Client _client() {
183183
}
184184

185185
@Specification(number="1.4.9", text="Methods, functions, or operations on the client MUST NOT throw exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the default value in the event of abnormal execution. Exceptions include functions or methods for the purposes for configuration or setup.")
186-
@Specification(number="1.4.7", text="In cases of abnormal execution, the evaluation details structure's error code field MUST contain a string identifying an error occurred during flag evaluation and the nature of the error.")
186+
@Specification(number="1.4.7", text="In cases of abnormal execution, the `evaluation details` structure's `error code` field **MUST** contain an `error code`.")
187+
@Specification(number="1.4.12", text="In cases of abnormal execution, the `evaluation details` structure's `error message` field **MAY** contain a string containing additional details about the nature of the error.")
187188
@Test void broken_provider() {
188189
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
189190
api.setProvider(new AlwaysBrokenProvider());
190191
Client c = api.getClient();
191192
assertFalse(c.getBooleanValue("key", false));
192193
FlagEvaluationDetails<Boolean> details = c.getBooleanDetails("key", false);
193-
assertEquals("BORK", details.getErrorCode());
194+
assertEquals(ErrorCode.FLAG_NOT_FOUND, details.getErrorCode());
195+
assertEquals(TestConstants.BROKEN_MESSAGE, details.getMessage());
194196
}
195197

196198
@Specification(number="1.4.10", text="In the case of abnormal execution, the client SHOULD log an informative error message.")
@@ -200,8 +202,8 @@ private Client _client() {
200202
api.setProvider(new AlwaysBrokenProvider());
201203
Client c = api.getClient();
202204
FlagEvaluationDetails<Boolean> result = c.getBooleanDetails("test", false);
203-
assertEquals(Reason.ERROR, result.getReason());
204-
assertThat(TEST_LOGGER.getLoggingEvents()).contains(LoggingEvent.error("Unable to correctly evaluate flag with key {} due to exception {}", "test", "BORK"));
205+
assertEquals(Reason.ERROR.toString(), result.getReason());
206+
assertThat(TEST_LOGGER.getLoggingEvents()).contains(LoggingEvent.error("Unable to correctly evaluate flag with key {} due to exception {}", "test", TestConstants.BROKEN_MESSAGE));
205207
}
206208

207209
@Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable name field or accessor of type string, which corresponds to the name value supplied during client creation.")
@@ -221,7 +223,7 @@ private Client _client() {
221223
api.setProvider(new AlwaysBrokenProvider());
222224
Client c = api.getClient();
223225
FlagEvaluationDetails<Boolean> result = c.getBooleanDetails("test", false);
224-
assertEquals(Reason.ERROR, result.getReason());
226+
assertEquals(Reason.ERROR.toString(), result.getReason());
225227
}
226228

227229
@Specification(number="3.2.1", text="The API, Client and invocation MUST have a method for supplying evaluation context.")

‎src/test/java/dev/openfeature/javasdk/ProviderSpecTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public class ProviderSpecTest {
4545
"reason field with a string indicating the semantic reason for the returned flag value.")
4646
@Test void has_reason() {
4747
ProviderEvaluation<Boolean> result = p.getBooleanEvaluation("key", false, new EvaluationContext());
48-
assertEquals(Reason.DEFAULT, result.getReason());
48+
assertEquals(Reason.DEFAULT.toString(), result.getReason());
4949
}
5050

5151
@Specification(number="2.7", text="In cases of normal execution, the provider MUST NOT populate " +
@@ -55,9 +55,9 @@ public class ProviderSpecTest {
5555
assertNull(result.getErrorCode());
5656
}
5757

58-
@Specification(number="2.8", text="In cases of abnormal execution, the provider MUST indicate an " +
59-
"error using the idioms of the implementation language, with an associated error code having possible " +
60-
"values PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, or GENERAL.")
58+
@Specification(number="2.8", text="In cases of abnormal execution, the `provider` **MUST** indicate an error using the idioms of the implementation language, with an associated `error code` and optional associated `error message`.")
59+
@Specification(number="2.11", text="In cases of normal execution, the `provider` **MUST NOT** populate the `flag resolution` structure's `error message` field, or otherwise must populate it with a null or falsy value.")
60+
@Specification(number="2.12", text="In cases of abnormal execution, the `evaluation details` structure's `error message` field **MAY** contain a string containing additional detail about the nature of the error.")
6161
@Test void up_to_provider_implementation() {}
6262

6363
@Specification(number="2.5", text="In cases of normal execution, the provider SHOULD populate the " +

‎src/test/java/dev/openfeature/javasdk/TestConstants.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package dev.openfeature.javasdk;
2+
3+
public class TestConstants {
4+
public static final String BROKEN_MESSAGE = "This is borked.";
5+
}

‎src/test/java/dev/openfeature/javasdk/integration/StepDefinitions.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertTrue;
5-
import static org.junit.jupiter.api.Assumptions.assumeFalse;
65

76
import dev.openfeature.contrib.providers.flagd.FlagdProvider;
87
import dev.openfeature.javasdk.Client;
9-
import dev.openfeature.javasdk.ErrorCode;
108
import dev.openfeature.javasdk.EvaluationContext;
119
import dev.openfeature.javasdk.FlagEvaluationDetails;
1210
import dev.openfeature.javasdk.OpenFeatureAPI;
@@ -132,7 +130,7 @@ public void the_resolved_boolean_value_should_be_the_variant_should_be_and_the_r
132130
String expectedVariant, String expectedReason) {
133131
assertEquals(Boolean.valueOf(expectedValue), booleanFlagDetails.getValue());
134132
assertEquals(expectedVariant, booleanFlagDetails.getVariant());
135-
assertEquals(Reason.valueOf(expectedReason), booleanFlagDetails.getReason());
133+
assertEquals(expectedReason, booleanFlagDetails.getReason());
136134
}
137135

138136
// string details
@@ -147,7 +145,7 @@ public void the_resolved_string_value_should_be_the_variant_should_be_and_the_re
147145
String expectedVariant, String expectedReason) {
148146
assertEquals(expectedValue, this.stringFlagDetails.getValue());
149147
assertEquals(expectedVariant, this.stringFlagDetails.getVariant());
150-
assertEquals(Reason.valueOf(expectedReason), this.stringFlagDetails.getReason());
148+
assertEquals(expectedReason, this.stringFlagDetails.getReason());
151149
}
152150

153151
// integer details
@@ -161,7 +159,7 @@ public void the_resolved_integer_value_should_be_the_variant_should_be_and_the_r
161159
String expectedVariant, String expectedReason) {
162160
assertEquals(expectedValue, this.intFlagDetails.getValue());
163161
assertEquals(expectedVariant, this.intFlagDetails.getVariant());
164-
assertEquals(Reason.valueOf(expectedReason), this.intFlagDetails.getReason());
162+
assertEquals(expectedReason, this.intFlagDetails.getReason());
165163
}
166164

167165
// float/double details
@@ -175,7 +173,7 @@ public void the_resolved_float_value_should_be_the_variant_should_be_and_the_rea
175173
String expectedVariant, String expectedReason) {
176174
assertEquals(expectedValue, this.doubleFlagDetails.getValue());
177175
assertEquals(expectedVariant, this.doubleFlagDetails.getVariant());
178-
assertEquals(Reason.valueOf(expectedReason), this.doubleFlagDetails.getReason());
176+
assertEquals(expectedReason, this.doubleFlagDetails.getReason());
179177
}
180178

181179
// object details
@@ -198,7 +196,7 @@ public void the_resolved_object_value_should_be_contain_fields_and_with_values_a
198196
@Then("the variant should be {string}, and the reason should be {string}")
199197
public void the_variant_should_be_and_the_reason_should_be(String expectedVariant, String expectedReason) {
200198
assertEquals(expectedVariant, this.objectFlagDetails.getVariant());
201-
assertEquals(Reason.valueOf(expectedReason), this.objectFlagDetails.getReason());
199+
assertEquals(expectedReason, this.objectFlagDetails.getReason());
202200
}
203201

204202
/*
@@ -255,8 +253,9 @@ public void then_the_default_string_value_should_be_returned() {
255253

256254
@Then("the reason should indicate an error and the error code should indicate a missing flag with {string}")
257255
public void the_reason_should_indicate_an_error_and_the_error_code_should_be_flag_not_found(String errorCode) {
258-
assertEquals(Reason.ERROR, notFoundDetails.getReason());
259-
assertTrue(notFoundDetails.getErrorCode().contains(errorCode));
256+
assertEquals(Reason.ERROR.toString(), notFoundDetails.getReason());
257+
assertTrue(notFoundDetails.getMessage().contains(errorCode));
258+
// TODO: add errorCode assertion once flagd provider is updated.
260259
}
261260

262261
// type mismatch
@@ -275,8 +274,9 @@ public void then_the_default_integer_value_should_be_returned() {
275274

276275
@Then("the reason should indicate an error and the error code should indicate a type mismatch with {string}")
277276
public void the_reason_should_indicate_an_error_and_the_error_code_should_be_type_mismatch(String errorCode) {
278-
assertEquals(Reason.ERROR, typeErrorDetails.getReason());
279-
assertTrue(typeErrorDetails.getErrorCode().contains(errorCode));
277+
assertEquals(Reason.ERROR.toString(), typeErrorDetails.getReason());
278+
assertTrue(typeErrorDetails.getMessage().contains(errorCode));
279+
// TODO: add errorCode assertion once flagd provider is updated.
280280
}
281281

282282
}

0 commit comments

Comments
 (0)