diff --git a/src/main/java/com/launchdarkly/client/Components.java b/src/main/java/com/launchdarkly/client/Components.java index fa6569186..5fcc53be2 100644 --- a/src/main/java/com/launchdarkly/client/Components.java +++ b/src/main/java/com/launchdarkly/client/Components.java @@ -113,7 +113,7 @@ public UpdateProcessor createUpdateProcessor(String sdkKey, LDConfig config, Fea FeatureRequestor requestor = new FeatureRequestor(sdkKey, config); if (config.stream) { logger.info("Enabling streaming API"); - return new StreamProcessor(sdkKey, config, requestor, featureStore); + return new StreamProcessor(sdkKey, config, requestor, featureStore, null); } else { logger.info("Disabling streaming API"); logger.warn("You should only disable the streaming API if instructed to do so by LaunchDarkly support"); diff --git a/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java b/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java index 9db6a7aa2..c165d8f9b 100644 --- a/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java +++ b/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java @@ -25,6 +25,8 @@ import java.util.concurrent.atomic.AtomicLong; import static com.launchdarkly.client.Util.getRequestBuilder; +import static com.launchdarkly.client.Util.httpErrorMessage; +import static com.launchdarkly.client.Util.isHttpErrorRecoverable; import okhttp3.MediaType; import okhttp3.Request; @@ -386,9 +388,12 @@ private void handleResponse(Response response) { } catch (ParseException e) { } } - if (response.code() == 401) { + if (!isHttpErrorRecoverable(response.code())) { disabled.set(true); - logger.error("Received 401 error, no further events will be posted since SDK key is invalid"); + logger.error(httpErrorMessage(response.code(), "posting events", "some events were dropped")); + // It's "some events were dropped" because we're not going to retry *this* request any more times - + // we only get to this point if we have used up our retry attempts. So the last batch of events was + // lost, even though we will still try to post *other* events in the future. } } } @@ -530,7 +535,7 @@ private void postEvents(List eventsOut) { logger.debug("Event delivery took {} ms, response status {}", endTime - startTime, response.code()); if (!response.isSuccessful()) { logger.warn("Unexpected response status when posting events: {}", response.code()); - if (response.code() >= 500) { + if (isHttpErrorRecoverable(response.code())) { continue; } } diff --git a/src/main/java/com/launchdarkly/client/FeatureRequestor.java b/src/main/java/com/launchdarkly/client/FeatureRequestor.java index 377f773c7..180270295 100644 --- a/src/main/java/com/launchdarkly/client/FeatureRequestor.java +++ b/src/main/java/com/launchdarkly/client/FeatureRequestor.java @@ -37,27 +37,27 @@ static class AllData { this.config = config; } - Map getAllFlags() throws IOException, InvalidSDKKeyException { + Map getAllFlags() throws IOException, HttpErrorException { String body = get(GET_LATEST_FLAGS_PATH); return FeatureFlag.fromJsonMap(config, body); } - FeatureFlag getFlag(String featureKey) throws IOException, InvalidSDKKeyException { + FeatureFlag getFlag(String featureKey) throws IOException, HttpErrorException { String body = get(GET_LATEST_FLAGS_PATH + "/" + featureKey); return FeatureFlag.fromJson(config, body); } - Map getAllSegments() throws IOException, InvalidSDKKeyException { + Map getAllSegments() throws IOException, HttpErrorException { String body = get(GET_LATEST_SEGMENTS_PATH); return Segment.fromJsonMap(config, body); } - Segment getSegment(String segmentKey) throws IOException, InvalidSDKKeyException { + Segment getSegment(String segmentKey) throws IOException, HttpErrorException { String body = get(GET_LATEST_SEGMENTS_PATH + "/" + segmentKey); return Segment.fromJson(config, body); } - AllData getAllData() throws IOException, InvalidSDKKeyException { + AllData getAllData() throws IOException, HttpErrorException { String body = get(GET_LATEST_ALL_PATH); return config.gson.fromJson(body, AllData.class); } @@ -69,7 +69,7 @@ AllData getAllData() throws IOException, InvalidSDKKeyException { return ret; } - private String get(String path) throws IOException, InvalidSDKKeyException { + private String get(String path) throws IOException, HttpErrorException { Request request = getRequestBuilder(sdkKey) .url(config.baseURI.toString() + path) .get() @@ -81,12 +81,7 @@ private String get(String path) throws IOException, InvalidSDKKeyException { String body = response.body().string(); if (!response.isSuccessful()) { - if (response.code() == 401) { - logger.error("[401] Invalid SDK key when accessing URI: " + request.url()); - throw new InvalidSDKKeyException(); - } - throw new IOException("Unexpected response when retrieving Feature Flag(s): " + response + " using url: " - + request.url() + " with body: " + body); + throw new HttpErrorException(response.code()); } logger.debug("Get flag(s) response: " + response.toString() + " with body: " + body); logger.debug("Network response: " + response.networkResponse()); @@ -98,10 +93,4 @@ private String get(String path) throws IOException, InvalidSDKKeyException { return body; } } - - @SuppressWarnings("serial") - public static class InvalidSDKKeyException extends Exception { - public InvalidSDKKeyException() { - } - } } \ No newline at end of file diff --git a/src/main/java/com/launchdarkly/client/HttpErrorException.java b/src/main/java/com/launchdarkly/client/HttpErrorException.java new file mode 100644 index 000000000..8f6672bbb --- /dev/null +++ b/src/main/java/com/launchdarkly/client/HttpErrorException.java @@ -0,0 +1,15 @@ +package com.launchdarkly.client; + +@SuppressWarnings("serial") +class HttpErrorException extends Exception { + private final int status; + + public HttpErrorException(int status) { + super("HTTP error " + status); + this.status = status; + } + + public int getStatus() { + return status; + } +} diff --git a/src/main/java/com/launchdarkly/client/LDClient.java b/src/main/java/com/launchdarkly/client/LDClient.java index 90c0f43ff..39d193272 100644 --- a/src/main/java/com/launchdarkly/client/LDClient.java +++ b/src/main/java/com/launchdarkly/client/LDClient.java @@ -97,6 +97,9 @@ public LDClient(String sdkKey, LDConfig config) { } catch (Exception e) { logger.error("Exception encountered waiting for LaunchDarkly client initialization", e); } + if (!updateProcessor.initialized()) { + logger.warn("LaunchDarkly client was not successfully initialized"); + } } } diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index bca375566..cc753e6e8 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -113,7 +113,7 @@ protected LDConfig(Builder builder) { .connectTimeout(connectTimeoutMillis, TimeUnit.MILLISECONDS) .readTimeout(socketTimeoutMillis, TimeUnit.MILLISECONDS) .writeTimeout(socketTimeoutMillis, TimeUnit.MILLISECONDS) - .retryOnConnectionFailure(true); + .retryOnConnectionFailure(false); // we will implement our own retry logic // When streaming is enabled, http GETs made by FeatureRequester will // always guarantee a new flag state. So, disable http response caching diff --git a/src/main/java/com/launchdarkly/client/LDUser.java b/src/main/java/com/launchdarkly/client/LDUser.java index 3887540bc..adde857bc 100644 --- a/src/main/java/com/launchdarkly/client/LDUser.java +++ b/src/main/java/com/launchdarkly/client/LDUser.java @@ -608,11 +608,7 @@ public Builder privateEmail(String email) { * @return the builder */ public Builder custom(String k, String v) { - checkCustomAttribute(k); - if (k != null && v != null) { - custom.put(k, new JsonPrimitive(v)); - } - return this; + return custom(k, v == null ? null : new JsonPrimitive(v)); } /** @@ -625,11 +621,7 @@ public Builder custom(String k, String v) { * @return the builder */ public Builder custom(String k, Number n) { - checkCustomAttribute(k); - if (k != null && n != null) { - custom.put(k, new JsonPrimitive(n)); - } - return this; + return custom(k, n == null ? null : new JsonPrimitive(n)); } /** @@ -642,9 +634,22 @@ public Builder custom(String k, Number n) { * @return the builder */ public Builder custom(String k, Boolean b) { + return custom(k, b == null ? null : new JsonPrimitive(b)); + } + + /** + * Add a custom attribute whose value can be any JSON type. When set to one of the + * built-in + * user attribute keys, this custom attribute will be ignored. + * + * @param k the key for the custom attribute + * @param v the value for the custom attribute + * @return the builder + */ + public Builder custom(String k, JsonElement v) { checkCustomAttribute(k); - if (k != null && b != null) { - custom.put(k, new JsonPrimitive(b)); + if (k != null && v != null) { + custom.put(k, v); } return this; } @@ -757,6 +762,21 @@ public Builder privateCustom(String k, Boolean b) { return custom(k, b); } + /** + * Add a custom attribute of any JSON type, that will not be sent back to LaunchDarkly. + * When set to one of the + * built-in + * user attribute keys, this custom attribute will be ignored. + * + * @param k the key for the custom attribute + * @param v the value for the custom attribute + * @return the builder + */ + public Builder privateCustom(String k, JsonElement v) { + privateAttrNames.add(k); + return custom(k, v); + } + /** * Add a list of {@link java.lang.String}-valued custom attributes. When set to one of the * diff --git a/src/main/java/com/launchdarkly/client/PollingProcessor.java b/src/main/java/com/launchdarkly/client/PollingProcessor.java index 1797ab474..bb261236a 100644 --- a/src/main/java/com/launchdarkly/client/PollingProcessor.java +++ b/src/main/java/com/launchdarkly/client/PollingProcessor.java @@ -2,13 +2,21 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.common.util.concurrent.ThreadFactoryBuilder; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.util.concurrent.*; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import static com.launchdarkly.client.Util.httpErrorMessage; +import static com.launchdarkly.client.Util.isHttpErrorRecoverable; + class PollingProcessor implements UpdateProcessor { private static final Logger logger = LoggerFactory.getLogger(PollingProcessor.class); @@ -55,9 +63,12 @@ public void run() { logger.info("Initialized LaunchDarkly client."); initFuture.set(null); } - } catch (FeatureRequestor.InvalidSDKKeyException e) { - logger.error("Received 401 error, no further polling requests will be made since SDK key is invalid"); - scheduler.shutdown(); + } catch (HttpErrorException e) { + logger.error(httpErrorMessage(e.getStatus(), "polling request", "will retry")); + if (!isHttpErrorRecoverable(e.getStatus())) { + scheduler.shutdown(); + initFuture.set(null); // if client is initializing, make it stop waiting; has no effect if already inited + } } catch (IOException e) { logger.error("Encountered exception in LaunchDarkly client when retrieving update", e); } diff --git a/src/main/java/com/launchdarkly/client/StreamProcessor.java b/src/main/java/com/launchdarkly/client/StreamProcessor.java index 713e928e3..1cdf5b7d1 100644 --- a/src/main/java/com/launchdarkly/client/StreamProcessor.java +++ b/src/main/java/com/launchdarkly/client/StreamProcessor.java @@ -1,17 +1,5 @@ package com.launchdarkly.client; -import static com.launchdarkly.client.VersionedDataKind.FEATURES; -import static com.launchdarkly.client.VersionedDataKind.SEGMENTS; - -import java.io.IOException; -import java.net.URI; -import java.util.concurrent.Future; -import java.util.concurrent.atomic.AtomicBoolean; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.SettableFuture; import com.google.gson.Gson; import com.google.gson.JsonElement; @@ -21,9 +9,22 @@ import com.launchdarkly.eventsource.MessageEvent; import com.launchdarkly.eventsource.UnsuccessfulResponseException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.net.URI; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicBoolean; + +import static com.launchdarkly.client.Util.httpErrorMessage; +import static com.launchdarkly.client.Util.isHttpErrorRecoverable; +import static com.launchdarkly.client.VersionedDataKind.FEATURES; +import static com.launchdarkly.client.VersionedDataKind.SEGMENTS; + import okhttp3.Headers; -class StreamProcessor implements UpdateProcessor { +final class StreamProcessor implements UpdateProcessor { private static final String PUT = "put"; private static final String PATCH = "patch"; private static final String DELETE = "delete"; @@ -36,15 +37,21 @@ class StreamProcessor implements UpdateProcessor { private final LDConfig config; private final String sdkKey; private final FeatureRequestor requestor; + private final EventSourceCreator eventSourceCreator; private volatile EventSource es; - private AtomicBoolean initialized = new AtomicBoolean(false); + private final AtomicBoolean initialized = new AtomicBoolean(false); - - StreamProcessor(String sdkKey, LDConfig config, FeatureRequestor requestor, FeatureStore featureStore) { + public static interface EventSourceCreator { + EventSource createEventSource(EventHandler handler, URI streamUri, ConnectionErrorHandler errorHandler, Headers headers); + } + + StreamProcessor(String sdkKey, LDConfig config, FeatureRequestor requestor, FeatureStore featureStore, + EventSourceCreator eventSourceCreator) { this.store = featureStore; this.config = config; this.sdkKey = sdkKey; this.requestor = requestor; + this.eventSourceCreator = eventSourceCreator != null ? eventSourceCreator : new DefaultEventSourceCreator(); } @Override @@ -60,10 +67,13 @@ public Future start() { ConnectionErrorHandler connectionErrorHandler = new ConnectionErrorHandler() { @Override public Action onConnectionError(Throwable t) { - if ((t instanceof UnsuccessfulResponseException) && - ((UnsuccessfulResponseException) t).getCode() == 401) { - logger.error("Received 401 error, no further streaming connection will be made since SDK key is invalid"); - return Action.SHUTDOWN; + if (t instanceof UnsuccessfulResponseException) { + int status = ((UnsuccessfulResponseException)t).getCode(); + logger.error(httpErrorMessage(status, "streaming connection", "will retry")); + if (!isHttpErrorRecoverable(status)) { + initFuture.set(null); // if client is initializing, make it stop waiting; has no effect if already inited + return Action.SHUTDOWN; + } } return Action.PROCEED; } @@ -162,7 +172,7 @@ public void onError(Throwable throwable) { } }; - es = createEventSource(handler, + es = eventSourceCreator.createEventSource(handler, URI.create(config.streamURI.toASCIIString() + "/all"), connectionErrorHandler, headers); @@ -170,30 +180,6 @@ public void onError(Throwable throwable) { return initFuture; } - @VisibleForTesting - protected EventSource createEventSource(EventHandler handler, URI streamUri, ConnectionErrorHandler errorHandler, - Headers headers) { - EventSource.Builder builder = new EventSource.Builder(handler, URI.create(config.streamURI.toASCIIString() + "/all")) - .connectionErrorHandler(errorHandler) - .headers(headers) - .reconnectTimeMs(config.reconnectTimeMs) - .connectTimeoutMs(config.connectTimeoutMillis) - .readTimeoutMs(DEAD_CONNECTION_INTERVAL_MS); - // Note that this is not the same read timeout that can be set in LDConfig. We default to a smaller one - // there because we don't expect long delays within any *non*-streaming response that the LD client gets. - // A read timeout on the stream will result in the connection being cycled, so we set this to be slightly - // more than the expected interval between heartbeat signals. - - if (config.proxy != null) { - builder.proxy(config.proxy); - if (config.proxyAuthenticator != null) { - builder.proxyAuthenticator(config.proxyAuthenticator); - } - } - - return builder.build(); - } - @Override public void close() throws IOException { logger.info("Closing LaunchDarkly StreamProcessor"); @@ -235,4 +221,28 @@ public DeleteData() { } } -} \ No newline at end of file + + private class DefaultEventSourceCreator implements EventSourceCreator { + public EventSource createEventSource(EventHandler handler, URI streamUri, ConnectionErrorHandler errorHandler, Headers headers) { + EventSource.Builder builder = new EventSource.Builder(handler, streamUri) + .connectionErrorHandler(errorHandler) + .headers(headers) + .reconnectTimeMs(config.reconnectTimeMs) + .connectTimeoutMs(config.connectTimeoutMillis) + .readTimeoutMs(DEAD_CONNECTION_INTERVAL_MS); + // Note that this is not the same read timeout that can be set in LDConfig. We default to a smaller one + // there because we don't expect long delays within any *non*-streaming response that the LD client gets. + // A read timeout on the stream will result in the connection being cycled, so we set this to be slightly + // more than the expected interval between heartbeat signals. + + if (config.proxy != null) { + builder.proxy(config.proxy); + if (config.proxyAuthenticator != null) { + builder.proxyAuthenticator(config.proxyAuthenticator); + } + } + + return builder.build(); + } + } +} diff --git a/src/main/java/com/launchdarkly/client/Util.java b/src/main/java/com/launchdarkly/client/Util.java index 3fca7e3c7..bb4bccd4b 100644 --- a/src/main/java/com/launchdarkly/client/Util.java +++ b/src/main/java/com/launchdarkly/client/Util.java @@ -32,4 +32,42 @@ static Request.Builder getRequestBuilder(String sdkKey) { .addHeader("Authorization", sdkKey) .addHeader("User-Agent", "JavaClient/" + LDClient.CLIENT_VERSION); } + + /** + * Tests whether an HTTP error status represents a condition that might resolve on its own if we retry. + * @param statusCode the HTTP status + * @return true if retrying makes sense; false if it should be considered a permanent failure + */ + static boolean isHttpErrorRecoverable(int statusCode) { + if (statusCode >= 400 && statusCode < 500) { + switch (statusCode) { + case 408: // request timeout + case 429: // too many requests + return true; + default: + return false; // all other 4xx errors are unrecoverable + } + } + return true; + } + + /** + * Builds an appropriate log message for an HTTP error status. + * @param statusCode the HTTP status + * @param context description of what we were trying to do + * @param recoverableMessage description of our behavior if the error is recoverable; typically "will retry" + * @return a message string + */ + static String httpErrorMessage(int statusCode, String context, String recoverableMessage) { + StringBuilder sb = new StringBuilder(); + sb.append("Received HTTP error ").append(statusCode); + switch (statusCode) { + case 401: + case 403: + sb.append(" (invalid SDK key)"); + } + sb.append(" for ").append(context).append(" - "); + sb.append(isHttpErrorRecoverable(statusCode) ? recoverableMessage : "giving up permanently"); + return sb.toString(); + } } diff --git a/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java b/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java index 8cc9eca54..d4b250e0a 100644 --- a/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java +++ b/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java @@ -8,7 +8,6 @@ import com.launchdarkly.client.DefaultEventProcessor.EventDispatcher; import org.hamcrest.Matcher; -import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -391,13 +390,43 @@ public void sdkKeyIsSent() throws Exception { assertThat(req.getHeader("Authorization"), equalTo(SDK_KEY)); } + + @Test + public void http401ErrorIsUnrecoverable() throws Exception { + testUnrecoverableHttpError(401); + } + + @Test + public void http403ErrorIsUnrecoverable() throws Exception { + testUnrecoverableHttpError(403); + } + + // Cannot test our retry logic for 408, because OkHttp insists on doing its own retry on 408 so that + // we never actually see that response status. +// @Test +// public void http408ErrorIsRecoverable() throws Exception { +// testRecoverableHttpError(408); +// } + + @Test + public void http429ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(429); + } + + @Test + public void http500ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(500); + } @Test - public void noMorePayloadsAreSentAfter401Error() throws Exception { + public void flushIsRetriedOnceAfter5xxError() throws Exception { + } + + private void testUnrecoverableHttpError(int status) throws Exception { ep = new DefaultEventProcessor(SDK_KEY, configBuilder.build()); Event e = EventFactory.DEFAULT.newIdentifyEvent(user); ep.sendEvent(e); - flushAndGetEvents(new MockResponse().setResponseCode(401)); + flushAndGetEvents(new MockResponse().setResponseCode(status)); ep.sendEvent(e); ep.flush(); @@ -405,15 +434,16 @@ public void noMorePayloadsAreSentAfter401Error() throws Exception { RecordedRequest req = server.takeRequest(0, TimeUnit.SECONDS); assertThat(req, nullValue(RecordedRequest.class)); } - - @Test - public void flushIsRetriedOnceAfter5xxError() throws Exception { + + private void testRecoverableHttpError(int status) throws Exception { ep = new DefaultEventProcessor(SDK_KEY, configBuilder.build()); Event e = EventFactory.DEFAULT.newIdentifyEvent(user); ep.sendEvent(e); - server.enqueue(new MockResponse().setResponseCode(503)); - server.enqueue(new MockResponse().setResponseCode(503)); + server.enqueue(new MockResponse().setResponseCode(status)); + server.enqueue(new MockResponse().setResponseCode(status)); + server.enqueue(new MockResponse()); + // need two responses because flush will be retried one time ep.flush(); ep.waitUntilInactive(); diff --git a/src/test/java/com/launchdarkly/client/LDClientTest.java b/src/test/java/com/launchdarkly/client/LDClientTest.java index 18add9a1a..cd50a1c57 100644 --- a/src/test/java/com/launchdarkly/client/LDClientTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientTest.java @@ -109,7 +109,7 @@ public void willWaitForUpdateProcessorIfWaitMillisIsNonZero() throws Exception { expect(updateProcessor.start()).andReturn(initFuture); expect(initFuture.get(10L, TimeUnit.MILLISECONDS)).andReturn(null); - expect(updateProcessor.initialized()).andReturn(false); + expect(updateProcessor.initialized()).andReturn(false).anyTimes(); replayAll(); client = createMockClient(config); @@ -125,7 +125,7 @@ public void updateProcessorCanTimeOut() throws Exception { expect(updateProcessor.start()).andReturn(initFuture); expect(initFuture.get(10L, TimeUnit.MILLISECONDS)).andThrow(new TimeoutException()); - expect(updateProcessor.initialized()).andReturn(false); + expect(updateProcessor.initialized()).andReturn(false).anyTimes(); replayAll(); client = createMockClient(config); @@ -141,7 +141,7 @@ public void clientCatchesRuntimeExceptionFromUpdateProcessor() throws Exception expect(updateProcessor.start()).andReturn(initFuture); expect(initFuture.get(10L, TimeUnit.MILLISECONDS)).andThrow(new RuntimeException()); - expect(updateProcessor.initialized()).andReturn(false); + expect(updateProcessor.initialized()).andReturn(false).anyTimes(); replayAll(); client = createMockClient(config); diff --git a/src/test/java/com/launchdarkly/client/LDUserTest.java b/src/test/java/com/launchdarkly/client/LDUserTest.java index cc4ad9cd7..ca8a9f7f4 100644 --- a/src/test/java/com/launchdarkly/client/LDUserTest.java +++ b/src/test/java/com/launchdarkly/client/LDUserTest.java @@ -19,6 +19,7 @@ import static com.launchdarkly.client.TestUtil.js; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; public class LDUserTest { @@ -241,6 +242,25 @@ public void getValueReturnsNullIfNotFound() { assertNull(user.getValueForEvaluation("height")); } + @Test + public void canAddCustomAttrWithJsonValue() { + JsonElement value = new JsonPrimitive("x"); + LDUser user = new LDUser.Builder("key") + .custom("foo", value) + .build(); + assertEquals(value, user.getCustom("foo")); + } + + @Test + public void canAddPrivateCustomAttrWithJsonValue() { + JsonElement value = new JsonPrimitive("x"); + LDUser user = new LDUser.Builder("key") + .privateCustom("foo", value) + .build(); + assertEquals(value, user.getCustom("foo")); + assertTrue(user.privateAttributeNames.contains("foo")); + } + @Test public void canAddCustomAttrWithListOfStrings() { LDUser user = new LDUser.Builder("key") diff --git a/src/test/java/com/launchdarkly/client/PollingProcessorTest.java b/src/test/java/com/launchdarkly/client/PollingProcessorTest.java index e42a26dd7..10d1a3722 100644 --- a/src/test/java/com/launchdarkly/client/PollingProcessorTest.java +++ b/src/test/java/com/launchdarkly/client/PollingProcessorTest.java @@ -53,4 +53,71 @@ public void testConnectionProblem() throws Exception { pollingProcessor.close(); verifyAll(); } + + @Test + public void http401ErrorIsUnrecoverable() throws Exception { + testUnrecoverableHttpError(401); + } + + @Test + public void http403ErrorIsUnrecoverable() throws Exception { + testUnrecoverableHttpError(403); + } + + @Test + public void http408ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(408); + } + + @Test + public void http429ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(429); + } + + @Test + public void http500ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(500); + } + + private void testUnrecoverableHttpError(int status) throws Exception { + FeatureRequestor requestor = createStrictMock(FeatureRequestor.class); + try (PollingProcessor pollingProcessor = new PollingProcessor(LDConfig.DEFAULT, requestor, new InMemoryFeatureStore())) { + expect(requestor.getAllData()) + .andThrow(new HttpErrorException(status)) + .once(); + replayAll(); + + long startTime = System.currentTimeMillis(); + Future initFuture = pollingProcessor.start(); + try { + initFuture.get(10, TimeUnit.SECONDS); + } catch (TimeoutException ignored) { + fail("Should not have timed out"); + } + assertTrue((System.currentTimeMillis() - startTime) < 9000); + assertTrue(initFuture.isDone()); + assertFalse(pollingProcessor.initialized()); + verifyAll(); + } + } + + private void testRecoverableHttpError(int status) throws Exception { + FeatureRequestor requestor = createStrictMock(FeatureRequestor.class); + try (PollingProcessor pollingProcessor = new PollingProcessor(LDConfig.DEFAULT, requestor, new InMemoryFeatureStore())) { + expect(requestor.getAllData()) + .andThrow(new HttpErrorException(status)) + .once(); + replayAll(); + + Future initFuture = pollingProcessor.start(); + try { + initFuture.get(200, TimeUnit.MILLISECONDS); + fail("expected timeout"); + } catch (TimeoutException ignored) { + } + assertFalse(initFuture.isDone()); + assertFalse(pollingProcessor.initialized()); + verifyAll(); + } + } } \ No newline at end of file diff --git a/src/test/java/com/launchdarkly/client/StreamProcessorTest.java b/src/test/java/com/launchdarkly/client/StreamProcessorTest.java index fbf452e7b..8b342359e 100644 --- a/src/test/java/com/launchdarkly/client/StreamProcessorTest.java +++ b/src/test/java/com/launchdarkly/client/StreamProcessorTest.java @@ -14,6 +14,8 @@ import java.net.URI; import java.util.Collections; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import static com.launchdarkly.client.TestUtil.specificFeatureStore; import static com.launchdarkly.client.VersionedDataKind.FEATURES; @@ -23,6 +25,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import okhttp3.Headers; @@ -279,36 +282,72 @@ public void streamWillReconnectAfterGeneralIOException() throws Exception { ConnectionErrorHandler.Action action = errorHandler.onConnectionError(new IOException()); assertEquals(ConnectionErrorHandler.Action.PROCEED, action); } + + @Test + public void http401ErrorIsUnrecoverable() throws Exception { + testUnrecoverableHttpError(401); + } @Test - public void streamWillReconnectAfterHttp500Error() throws Exception { - createStreamProcessor(SDK_KEY, configBuilder.build()).start(); - UnsuccessfulResponseException e = new UnsuccessfulResponseException(500); - ConnectionErrorHandler.Action action = errorHandler.onConnectionError(e); - assertEquals(ConnectionErrorHandler.Action.PROCEED, action); + public void http403ErrorIsUnrecoverable() throws Exception { + testUnrecoverableHttpError(403); } @Test - public void streamWillCloseAfterHttp401Error() throws Exception { - createStreamProcessor(SDK_KEY, configBuilder.build()).start(); - UnsuccessfulResponseException e = new UnsuccessfulResponseException(401); + public void http408ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(408); + } + + @Test + public void http429ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(429); + } + + @Test + public void http500ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(500); + } + + private void testUnrecoverableHttpError(int status) throws Exception { + UnsuccessfulResponseException e = new UnsuccessfulResponseException(status); + long startTime = System.currentTimeMillis(); + StreamProcessor sp = createStreamProcessor(SDK_KEY, configBuilder.build()); + Future initFuture = sp.start(); + ConnectionErrorHandler.Action action = errorHandler.onConnectionError(e); assertEquals(ConnectionErrorHandler.Action.SHUTDOWN, action); + + try { + initFuture.get(10, TimeUnit.SECONDS); + } catch (TimeoutException ignored) { + fail("Should not have timed out"); + } + assertTrue((System.currentTimeMillis() - startTime) < 9000); + assertTrue(initFuture.isDone()); + assertFalse(sp.initialized()); } - + + private void testRecoverableHttpError(int status) throws Exception { + UnsuccessfulResponseException e = new UnsuccessfulResponseException(status); + long startTime = System.currentTimeMillis(); + StreamProcessor sp = createStreamProcessor(SDK_KEY, configBuilder.build()); + Future initFuture = sp.start(); + + ConnectionErrorHandler.Action action = errorHandler.onConnectionError(e); + assertEquals(ConnectionErrorHandler.Action.PROCEED, action); + + try { + initFuture.get(200, TimeUnit.MILLISECONDS); + fail("Expected timeout"); + } catch (TimeoutException ignored) { + } + assertTrue((System.currentTimeMillis() - startTime) >= 200); + assertFalse(initFuture.isDone()); + assertFalse(sp.initialized()); + } + private StreamProcessor createStreamProcessor(String sdkKey, LDConfig config) { - return new StreamProcessor(sdkKey, config, mockRequestor, featureStore) { - @Override - protected EventSource createEventSource(EventHandler handler, URI streamUri, ConnectionErrorHandler errorHandler, - Headers headers) { - - StreamProcessorTest.this.eventHandler = handler; - StreamProcessorTest.this.actualStreamUri = streamUri; - StreamProcessorTest.this.errorHandler = errorHandler; - StreamProcessorTest.this.headers = headers; - return mockEventSource; - } - }; + return new StreamProcessor(sdkKey, config, mockRequestor, featureStore, new StubEventSourceCreator()); } private String featureJson(String key, int version) { @@ -336,4 +375,15 @@ private void assertFeatureInStore(FeatureFlag feature) { private void assertSegmentInStore(Segment segment) { assertEquals(segment.getVersion(), featureStore.get(SEGMENTS, segment.getKey()).getVersion()); } + + private class StubEventSourceCreator implements StreamProcessor.EventSourceCreator { + public EventSource createEventSource(EventHandler handler, URI streamUri, ConnectionErrorHandler errorHandler, + Headers headers) { + StreamProcessorTest.this.eventHandler = handler; + StreamProcessorTest.this.actualStreamUri = streamUri; + StreamProcessorTest.this.errorHandler = errorHandler; + StreamProcessorTest.this.headers = headers; + return mockEventSource; + } + } }