From 3d95d43fd7769d998038ea5079d0b637c57c96ac Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 28 Jun 2018 15:01:44 -0700 Subject: [PATCH 1/6] don't give up permanently after a 400 error --- src/main/java/com/launchdarkly/client/Util.java | 1 + .../com/launchdarkly/client/DefaultEventProcessorTest.java | 5 +++++ .../java/com/launchdarkly/client/PollingProcessorTest.java | 5 +++++ .../java/com/launchdarkly/client/StreamProcessorTest.java | 5 +++++ 4 files changed, 16 insertions(+) diff --git a/src/main/java/com/launchdarkly/client/Util.java b/src/main/java/com/launchdarkly/client/Util.java index bb4bccd4b..78ea7b759 100644 --- a/src/main/java/com/launchdarkly/client/Util.java +++ b/src/main/java/com/launchdarkly/client/Util.java @@ -41,6 +41,7 @@ static Request.Builder getRequestBuilder(String sdkKey) { static boolean isHttpErrorRecoverable(int statusCode) { if (statusCode >= 400 && statusCode < 500) { switch (statusCode) { + case 400: // bad request case 408: // request timeout case 429: // too many requests return true; diff --git a/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java b/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java index d4b250e0a..36bc2097e 100644 --- a/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java +++ b/src/test/java/com/launchdarkly/client/DefaultEventProcessorTest.java @@ -391,6 +391,11 @@ public void sdkKeyIsSent() throws Exception { assertThat(req.getHeader("Authorization"), equalTo(SDK_KEY)); } + @Test + public void http400ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(400); + } + @Test public void http401ErrorIsUnrecoverable() throws Exception { testUnrecoverableHttpError(401); diff --git a/src/test/java/com/launchdarkly/client/PollingProcessorTest.java b/src/test/java/com/launchdarkly/client/PollingProcessorTest.java index 10d1a3722..dc18f1421 100644 --- a/src/test/java/com/launchdarkly/client/PollingProcessorTest.java +++ b/src/test/java/com/launchdarkly/client/PollingProcessorTest.java @@ -53,6 +53,11 @@ public void testConnectionProblem() throws Exception { pollingProcessor.close(); verifyAll(); } + + @Test + public void http400ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(400); + } @Test public void http401ErrorIsUnrecoverable() throws Exception { diff --git a/src/test/java/com/launchdarkly/client/StreamProcessorTest.java b/src/test/java/com/launchdarkly/client/StreamProcessorTest.java index 8b342359e..ee29937c9 100644 --- a/src/test/java/com/launchdarkly/client/StreamProcessorTest.java +++ b/src/test/java/com/launchdarkly/client/StreamProcessorTest.java @@ -282,6 +282,11 @@ public void streamWillReconnectAfterGeneralIOException() throws Exception { ConnectionErrorHandler.Action action = errorHandler.onConnectionError(new IOException()); assertEquals(ConnectionErrorHandler.Action.PROCEED, action); } + + @Test + public void http400ErrorIsRecoverable() throws Exception { + testRecoverableHttpError(400); + } @Test public void http401ErrorIsUnrecoverable() throws Exception { From afdb396ef78d4220affd45718ddd8da56a1c2bac Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 2 Jul 2018 17:47:29 -0700 Subject: [PATCH 2/6] add brief Java compatibility note to readme --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 9f7c2f290..876ef8950 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,11 @@ LaunchDarkly SDK for Java [![Javadocs](http://javadoc.io/badge/com.launchdarkly/launchdarkly-client.svg)](http://javadoc.io/doc/com.launchdarkly/launchdarkly-client) [![FOSSA Status](https://app.fossa.io/api/projects/git%2Bhttps%3A%2F%2Fgithub.com%2Flaunchdarkly%2Fjava-client.svg?type=shield)](https://app.fossa.io/projects/git%2Bhttps%3A%2F%2Fgithub.com%2Flaunchdarkly%2Fjava-client?ref=badge_shield) +Supported Java versions +----------------------- + +This version of the LaunchDarkly SDK works with Java 7 and above. + Quick setup ----------- From b9e06344cba629593e84257382253c7d8bccdea0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 13 Jul 2018 17:02:46 -0700 Subject: [PATCH 3/6] avoid unnecessary retry after Redis update --- src/main/java/com/launchdarkly/client/RedisFeatureStore.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java index d4724a40f..1a9c1935a 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java @@ -259,6 +259,7 @@ private void updateItemWithVersioning(VersionedDataKin if (cache != null) { cache.invalidate(new CacheKey(kind, newItem.getKey())); } + return; } finally { if (jedis != null) { jedis.unwatch(); From 2bae5ab2ca7ea2100bcd64a8aeda5eabacdaa891 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 16 Aug 2018 10:59:35 -0700 Subject: [PATCH 4/6] better error logging practices --- .../java/com/launchdarkly/client/Clause.java | 6 +-- .../client/DefaultEventProcessor.java | 8 ++-- .../com/launchdarkly/client/FeatureFlag.java | 4 +- .../com/launchdarkly/client/LDClient.java | 38 ++++++++++++------- .../client/NewRelicReflector.java | 3 +- .../launchdarkly/client/PollingProcessor.java | 3 +- .../launchdarkly/client/StreamProcessor.java | 10 +++-- 7 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/Clause.java b/src/main/java/com/launchdarkly/client/Clause.java index 65cb756dc..2f69bf79b 100644 --- a/src/main/java/com/launchdarkly/client/Clause.java +++ b/src/main/java/com/launchdarkly/client/Clause.java @@ -38,7 +38,7 @@ boolean matchesUserNoSegments(LDUser user) { JsonArray array = userValue.getAsJsonArray(); for (JsonElement jsonElement : array) { if (!jsonElement.isJsonPrimitive()) { - logger.error("Invalid custom attribute value in user object: " + jsonElement); + logger.error("Invalid custom attribute value in user object for user key \"{}\": {}", user.getKey(), jsonElement); return false; } if (matchAny(jsonElement.getAsJsonPrimitive())) { @@ -49,8 +49,8 @@ boolean matchesUserNoSegments(LDUser user) { } else if (userValue.isJsonPrimitive()) { return maybeNegate(matchAny(userValue.getAsJsonPrimitive())); } - logger.warn("Got unexpected user attribute type: " + userValue.getClass().getName() + " for user key: " - + user.getKey() + " and attribute: " + attribute); + logger.warn("Got unexpected user attribute type \"{}\" for user key \"{}\" and attribute \"{}\"", + userValue.getClass().getName(), user.getKey(), attribute); return false; } diff --git a/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java b/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java index c165d8f9b..eed973fa2 100644 --- a/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java +++ b/src/main/java/com/launchdarkly/client/DefaultEventProcessor.java @@ -255,8 +255,8 @@ private void runMainLoop(BlockingQueue inputChannel, message.completed(); } catch (InterruptedException e) { } catch (Exception e) { - logger.error("Unexpected error in event processor: " + e); - logger.debug(e.getMessage(), e); + logger.error("Unexpected error in event processor: {}", e.toString()); + logger.debug(e.toString(), e); } } } @@ -493,8 +493,8 @@ public void run() { postEvents(eventsOut); } } catch (Exception e) { - logger.error("Unexpected error in event processor: " + e); - logger.debug(e.getMessage(), e); + logger.error("Unexpected error in event processor: {}", e.toString()); + logger.debug(e.toString(), e); } synchronized (activeFlushWorkersCount) { activeFlushWorkersCount.decrementAndGet(); diff --git a/src/main/java/com/launchdarkly/client/FeatureFlag.java b/src/main/java/com/launchdarkly/client/FeatureFlag.java index faff33c6b..2b06efe03 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlag.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlag.java @@ -65,7 +65,7 @@ EvalResult evaluate(LDUser user, FeatureStore featureStore, EventFactory eventFa List prereqEvents = new ArrayList<>(); if (user == null || user.getKey() == null) { - logger.warn("Null user or null user key when evaluating flag: " + key + "; returning null"); + logger.warn("Null user or null user key when evaluating flag \"{}\"; returning null", key); return new EvalResult(null, prereqEvents); } @@ -87,7 +87,7 @@ private VariationAndValue evaluate(LDUser user, FeatureStore featureStore, List< FeatureFlag prereqFeatureFlag = featureStore.get(FEATURES, prereq.getKey()); VariationAndValue prereqEvalResult = null; if (prereqFeatureFlag == null) { - logger.error("Could not retrieve prerequisite flag: " + prereq.getKey() + " when evaluating: " + key); + logger.error("Could not retrieve prerequisite flag \"{}\" when evaluating \"{}\"", prereq.getKey(), key); return null; } else if (prereqFeatureFlag.isOn()) { prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, events, eventFactory); diff --git a/src/main/java/com/launchdarkly/client/LDClient.java b/src/main/java/com/launchdarkly/client/LDClient.java index 39d193272..9dba6eca0 100644 --- a/src/main/java/com/launchdarkly/client/LDClient.java +++ b/src/main/java/com/launchdarkly/client/LDClient.java @@ -95,7 +95,8 @@ public LDClient(String sdkKey, LDConfig config) { } catch (TimeoutException e) { logger.error("Timeout encountered waiting for LaunchDarkly client initialization"); } catch (Exception e) { - logger.error("Exception encountered waiting for LaunchDarkly client initialization", e); + logger.error("Exception encountered waiting for LaunchDarkly client initialization: {}", e.toString()); + logger.debug(e.toString(), e); } if (!updateProcessor.initialized()) { logger.warn("LaunchDarkly client was not successfully initialized"); @@ -169,7 +170,8 @@ public Map allFlags(LDUser user) { result.put(entry.getKey(), evalResult); } catch (EvaluationException e) { - logger.error("Exception caught when evaluating all flags:", e); + logger.error("Exception caught for feature flag \"{}\" when evaluating all flags: {}", entry.getKey(), e.toString()); + logger.debug(e.toString(), e); } } return result; @@ -209,9 +211,9 @@ public JsonElement jsonVariation(String featureKey, LDUser user, JsonElement def public boolean isFlagKnown(String featureKey) { if (!initialized()) { if (featureStore.initialized()) { - logger.warn("isFlagKnown called before client initialized for feature flag " + featureKey + "; using last known values from feature store"); + logger.warn("isFlagKnown called before client initialized for feature flag \"{}\"; using last known values from feature store", featureKey); } else { - logger.warn("isFlagKnown called before client initialized for feature flag " + featureKey + "; feature store unavailable, returning false"); + logger.warn("isFlagKnown called before client initialized for feature flag \"{}\"; feature store unavailable, returning false", featureKey); return false; } } @@ -221,7 +223,8 @@ public boolean isFlagKnown(String featureKey) { return true; } } catch (Exception e) { - logger.error("Encountered exception in LaunchDarkly client", e); + logger.error("Encountered exception while calling isFlagKnown for feature flag \"{}\": {}", e.toString()); + logger.debug(e.toString(), e); } return false; @@ -230,23 +233,24 @@ public boolean isFlagKnown(String featureKey) { private JsonElement evaluate(String featureKey, LDUser user, JsonElement defaultValue, VariationType expectedType) { if (!initialized()) { if (featureStore.initialized()) { - logger.warn("Evaluation called before client initialized for feature flag " + featureKey + "; using last known values from feature store"); + logger.warn("Evaluation called before client initialized for feature flag \"{}\"; using last known values from feature store", featureKey); } else { - logger.warn("Evaluation called before client initialized for feature flag " + featureKey + "; feature store unavailable, returning default value"); + logger.warn("Evaluation called before client initialized for feature flag \"{}\"; feature store unavailable, returning default value", featureKey); sendFlagRequestEvent(eventFactory.newUnknownFeatureRequestEvent(featureKey, user, defaultValue)); return defaultValue; } } + FeatureFlag featureFlag = null; try { - FeatureFlag featureFlag = featureStore.get(FEATURES, featureKey); + featureFlag = featureStore.get(FEATURES, featureKey); if (featureFlag == null) { - logger.info("Unknown feature flag " + featureKey + "; returning default value"); + logger.info("Unknown feature flag \"{}\"; returning default value", featureKey); sendFlagRequestEvent(eventFactory.newUnknownFeatureRequestEvent(featureKey, user, defaultValue)); return defaultValue; } if (user == null || user.getKey() == null) { - logger.warn("Null user or null user key when evaluating flag: " + featureKey + "; returning default value"); + logger.warn("Null user or null user key when evaluating flag \"{}\"; returning default value", featureKey); sendFlagRequestEvent(eventFactory.newDefaultFeatureRequestEvent(featureFlag, user, defaultValue)); return defaultValue; } @@ -266,10 +270,15 @@ private JsonElement evaluate(String featureKey, LDUser user, JsonElement default return defaultValue; } } catch (Exception e) { - logger.error("Encountered exception in LaunchDarkly client", e); + logger.error("Encountered exception while evaluating feature flag \"{}\": {}", featureKey, e.toString()); + logger.debug(e.toString(), e); + if (featureFlag == null) { + sendFlagRequestEvent(eventFactory.newUnknownFeatureRequestEvent(featureKey, user, defaultValue)); + } else { + sendFlagRequestEvent(eventFactory.newDefaultFeatureRequestEvent(featureFlag, user, defaultValue)); + } + return defaultValue; } - sendFlagRequestEvent(eventFactory.newUnknownFeatureRequestEvent(featureKey, user, defaultValue)); - return defaultValue; } @Override @@ -314,7 +323,8 @@ public String secureModeHash(LDUser user) { mac.init(new SecretKeySpec(sdkKey.getBytes(), HMAC_ALGORITHM)); return Hex.encodeHexString(mac.doFinal(user.getKeyAsString().getBytes("UTF8"))); } catch (InvalidKeyException | UnsupportedEncodingException | NoSuchAlgorithmException e) { - logger.error("Could not generate secure mode hash", e); + logger.error("Could not generate secure mode hash: {}", e.toString()); + logger.debug(e.toString(), e); } return null; } diff --git a/src/main/java/com/launchdarkly/client/NewRelicReflector.java b/src/main/java/com/launchdarkly/client/NewRelicReflector.java index f01832d6f..8a9c1c0cb 100644 --- a/src/main/java/com/launchdarkly/client/NewRelicReflector.java +++ b/src/main/java/com/launchdarkly/client/NewRelicReflector.java @@ -28,7 +28,8 @@ static void annotateTransaction(String featureKey, String value) { try { addCustomParameter.invoke(null, featureKey, value); } catch (Exception e) { - logger.error("Unexpected error in LaunchDarkly NewRelic integration"); + logger.error("Unexpected error in LaunchDarkly NewRelic integration: {}", e.toString()); + logger.debug(e.toString(), e); } } } diff --git a/src/main/java/com/launchdarkly/client/PollingProcessor.java b/src/main/java/com/launchdarkly/client/PollingProcessor.java index bb261236a..ad8fdaead 100644 --- a/src/main/java/com/launchdarkly/client/PollingProcessor.java +++ b/src/main/java/com/launchdarkly/client/PollingProcessor.java @@ -70,7 +70,8 @@ public void run() { 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); + logger.error("Encountered exception in LaunchDarkly client when retrieving update: {}", e.toString()); + logger.debug(e.toString(), e); } } }, 0L, config.pollingIntervalMillis, TimeUnit.MILLISECONDS); diff --git a/src/main/java/com/launchdarkly/client/StreamProcessor.java b/src/main/java/com/launchdarkly/client/StreamProcessor.java index 1cdf5b7d1..6c98f4ee0 100644 --- a/src/main/java/com/launchdarkly/client/StreamProcessor.java +++ b/src/main/java/com/launchdarkly/client/StreamProcessor.java @@ -133,7 +133,8 @@ public void onMessage(String name, MessageEvent event) throws Exception { logger.info("Initialized LaunchDarkly client."); } } catch (IOException e) { - logger.error("Encountered exception in LaunchDarkly client", e); + logger.error("Encountered exception in LaunchDarkly client: {}", e.toString()); + logger.debug(e.toString(), e); } break; case INDIRECT_PATCH: @@ -151,7 +152,8 @@ public void onMessage(String name, MessageEvent event) throws Exception { } } } catch (IOException e) { - logger.error("Encountered exception in LaunchDarkly client", e); + logger.error("Encountered exception in LaunchDarkly client: {}", e.toString()); + logger.debug(e.toString(), e); } break; default: @@ -167,8 +169,8 @@ public void onComment(String comment) { @Override public void onError(Throwable throwable) { - logger.error("Encountered EventSource error: " + throwable.getMessage()); - logger.debug("", throwable); + logger.error("Encountered EventSource error: {}" + throwable.toString()); + logger.debug(throwable.toString(), throwable); } }; From a4a56953d2447b0193a455305c7bcf153d6525d0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 16 Aug 2018 14:05:55 -0700 Subject: [PATCH 5/6] remove FOSSA upload step from CI --- .circleci/config.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2da9f6d9d..d938b76e3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -22,11 +22,3 @@ jobs: path: ~/junit - store_artifacts: path: ~/junit - - run: - name: Upload FOSSA analysis (from master only) - command: | - if [[ ( -n "$FOSSA_API_KEY" ) && ( "$CIRCLE_BRANCH" == "master" ) ]]; then - curl -s -H 'Cache-Control: no-cache' https://raw.githubusercontent.com/fossas/fossa-cli/master/install.sh | sudo bash; - fossa init; - FOSSA_API_KEY=$FOSSA_API_KEY fossa; - fi From a7b9b16de13fccb52fa2f75aa2d33e5e74d1fe48 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 17 Aug 2018 10:51:35 -0700 Subject: [PATCH 6/6] version 4.2.2 --- CHANGELOG.md | 5 +++++ gradle.properties | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 852f409bf..f23b75b19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ All notable changes to the LaunchDarkly Java SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [4.2.2] - 2018-08-17 +### Fixed: +- When logging errors related to the evaluation of a specific flag, the log message now always includes the flag key. +- Exception stacktraces are now logged only at DEBUG level. Previously, some were being logged at ERROR level. + ## [4.2.1] - 2018-07-16 ### Fixed: - Should not permanently give up on posting events if the server returns a 400 error. diff --git a/gradle.properties b/gradle.properties index ebf7f6ded..e95b27cb1 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=4.2.1 +version=4.2.2 ossrhUsername= ossrhPassword=