From 350243243c018a1ef73ed30ed800bcbba46f0aba Mon Sep 17 00:00:00 2001 From: CaitlynStocker Date: Thu, 28 May 2026 11:59:19 +1000 Subject: [PATCH 1/5] Remove 5 second retry from context provider --- .../provider/OctopusContextProvider.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java index ca56e1f..0f0ec88 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java @@ -1,7 +1,5 @@ package com.octopus.openfeature.provider; -import java.time.Duration; - class OctopusContextProvider { private final OctopusConfiguration config; private final OctopusClient client; @@ -9,7 +7,6 @@ class OctopusContextProvider { private OctopusContext currentContext = OctopusContext.empty(); private Thread refreshThread; private static final System.Logger logger = System.getLogger(OctopusClient.class.getName()); - private static final Duration retryDelay = Duration.ofSeconds(5); OctopusContextProvider(OctopusConfiguration config, OctopusClient client) { this.config = config; @@ -44,11 +41,9 @@ void initialize() { otherwise the state will be left stale whilst the consumer continues to make use it. */ void refresh() { - int retryAttempt = 0; - var delay = config.getCacheDuration(); while (!Thread.currentThread().isInterrupted()) { try { - Thread.sleep(delay.toMillis()); + Thread.sleep(config.getCacheDuration().toMillis()); if (client.haveFeatureTogglesChanged(currentContext.getContentHash())) { var toggles = client.getFeatureToggleEvaluationManifest(); @@ -58,17 +53,11 @@ void refresh() { logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature manifest. Retaining existing context which may be stale."); } } - - delay = config.getCacheDuration(); - retryAttempt = 0; - } catch (InterruptedException e) { // the loop will be terminated and the thread will finish Thread.currentThread().interrupt(); } catch (Exception e) { - logger.log(System.Logger.Level.ERROR, String.format("Failed to refresh feature toggles. Retry attempt %d", retryAttempt), e); - delay = retryDelay; - retryAttempt++; + logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature manifest. Retaining existing context which may be stale.", e); } } } From 8a7d55306e064b2f16c4fc29f9612b64cab939b1 Mon Sep 17 00:00:00 2001 From: CaitlynStocker Date: Thu, 28 May 2026 12:43:01 +1000 Subject: [PATCH 2/5] Add tests --- .../provider/OctopusContextProviderTests.java | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java index 6fbe61f..1b1f839 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java @@ -121,4 +121,139 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws provider.shutdown(); } } + + @Test + void whenInitialFetchReturnsNothing_AndRefreshSucceeds_ContextIsPopulated() throws InterruptedException { + + byte[] contentHash = {0x01, 0x02, 0x03, 0x04}; + + // Initialize with null so first fetch fails + var client = new MockOctopusFeatureClient(null); + var provider = new OctopusContextProvider(configuration, client); + provider.initialize(); + + try { + // Check that the context is empty + assertThat(provider.getOctopusContext().getContentHash()).isEmpty(); + + // Update client to return valid toggles and wait for refresh + client.changeToggles(new FeatureToggles( + List.of(new FeatureToggleEvaluation("test-feature", false, "evaluation-key", Collections.emptyList(), 100)), + contentHash + )); + Thread.sleep(5000); + + // Assert that the context is now correctly populated + assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(contentHash); + + } finally { + provider.shutdown(); + } + } + + @Test + void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() throws InterruptedException { + + byte[] initialHash = {0x01, 0x02, 0x03, 0x04}; + byte[] updatedHash = {0x01, 0x02, 0x03, 0x05}; + + var logMessages = new ArrayList(); + var julLogger = Logger.getLogger(OctopusClient.class.getName()); + var handler = new Handler() { + @Override public void publish(LogRecord record) { logMessages.add(record.getMessage()); } + @Override public void flush() {} + @Override public void close() {} + }; + julLogger.addHandler(handler); + + // initialize with a client that returns valid toggles + var client = new MockOctopusFeatureClient(new FeatureToggles( + List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), + initialHash + )); + var provider = new OctopusContextProvider(configuration, client); + provider.initialize(); + + try { + // Switch to a null client and wait for refresh to fail + client.changeToggles(null); + Thread.sleep(5000); + + // Assert that failed refresh is logged and old context is retained + assertThat(logMessages).anyMatch(m -> m.startsWith("Failed to retrieve updated feature manifest")); + assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(initialHash); + + // Update client to return valid toggles again and wait for refresh + client.changeToggles(new FeatureToggles( + List.of(new FeatureToggleEvaluation("test-feature", false, "evaluation-key", Collections.emptyList(), 100)), + updatedHash + )); + Thread.sleep(5000); + + assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(updatedHash); + + } finally { + julLogger.removeHandler(handler); + provider.shutdown(); + } + } + + static class ThrowsOnRefreshClient extends OctopusClient { + + static final String ERROR_MESSAGE = "Oops! Simulated refresh error"; + private final FeatureToggles initial; + + ThrowsOnRefreshClient(FeatureToggles initial) { + super(null); + this.initial = initial; + } + + @Override + Boolean haveFeatureTogglesChanged(byte[] contentHash) { + throw new RuntimeException(ERROR_MESSAGE); + } + + @Override + FeatureToggles getFeatureToggleEvaluationManifest() { + return initial; + } + } + + @Test + void whenAnExceptionIsThrownDuringRefresh_LogsErrorDetails() throws InterruptedException { + + byte[] contentHash = {0x01, 0x02, 0x03, 0x04}; + + var logRecords = new ArrayList(); + var julLogger = Logger.getLogger(OctopusClient.class.getName()); + var handler = new Handler() { + @Override public void publish(LogRecord record) { logRecords.add(record); } + @Override public void flush() {} + @Override public void close() {} + }; + julLogger.addHandler(handler); + + // Initialize with a client that will throw on refresh + var client = new ThrowsOnRefreshClient(new FeatureToggles( + List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), + contentHash + )); + var provider = new OctopusContextProvider(configuration, client); + provider.initialize(); + + try { + // Wait for cache to expire and refresh attempt to throw + Thread.sleep(500); + + assertThat(logRecords).anyMatch(r -> + r.getMessage().startsWith("Failed to retrieve updated feature manifest") + && r.getThrown() != null + && r.getThrown().getMessage().contains(ThrowsOnRefreshClient.ERROR_MESSAGE) + ); + + } finally { + julLogger.removeHandler(handler); + provider.shutdown(); + } + } } From 4ea2f9be44bba5f5eb6fcd8100ddefb5dca7c30c Mon Sep 17 00:00:00 2001 From: CaitlynStocker Date: Thu, 28 May 2026 14:19:33 +1000 Subject: [PATCH 3/5] Hide scary logging from refresh failures in tests --- .../provider/OctopusContextProviderTests.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java index 1b1f839..392f186 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java @@ -105,6 +105,9 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws try { provider.initialize(); + // Stop warnings about refresh failures from reaching the test output + julLogger.setUseParentHandlers(false); + // Simulate a failed fetch client.changeToggles(null); @@ -118,6 +121,7 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws } finally { julLogger.removeHandler(handler); + julLogger.setUseParentHandlers(true); provider.shutdown(); } } @@ -127,6 +131,9 @@ void whenInitialFetchReturnsNothing_AndRefreshSucceeds_ContextIsPopulated() thro byte[] contentHash = {0x01, 0x02, 0x03, 0x04}; + var julLogger = Logger.getLogger(OctopusClient.class.getName()); + julLogger.setUseParentHandlers(false); + // Initialize with null so first fetch fails var client = new MockOctopusFeatureClient(null); var provider = new OctopusContextProvider(configuration, client); @@ -147,6 +154,7 @@ void whenInitialFetchReturnsNothing_AndRefreshSucceeds_ContextIsPopulated() thro assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(contentHash); } finally { + julLogger.setUseParentHandlers(true); provider.shutdown(); } } @@ -174,6 +182,9 @@ void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() t var provider = new OctopusContextProvider(configuration, client); provider.initialize(); + // Stop warnings about refresh failures from reaching the test output + julLogger.setUseParentHandlers(false); + try { // Switch to a null client and wait for refresh to fail client.changeToggles(null); @@ -194,6 +205,7 @@ void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() t } finally { julLogger.removeHandler(handler); + julLogger.setUseParentHandlers(true); provider.shutdown(); } } @@ -232,6 +244,8 @@ void whenAnExceptionIsThrownDuringRefresh_LogsErrorDetails() throws InterruptedE @Override public void close() {} }; julLogger.addHandler(handler); + // Stop warnings about refresh failures from reaching the test output + julLogger.setUseParentHandlers(false); // Initialize with a client that will throw on refresh var client = new ThrowsOnRefreshClient(new FeatureToggles( @@ -253,6 +267,7 @@ void whenAnExceptionIsThrownDuringRefresh_LogsErrorDetails() throws InterruptedE } finally { julLogger.removeHandler(handler); + julLogger.setUseParentHandlers(true); provider.shutdown(); } } From 111a4be28131198460e9251ff8cbd255afb0c58a Mon Sep 17 00:00:00 2001 From: CaitlynStocker Date: Fri, 29 May 2026 13:38:05 +1000 Subject: [PATCH 4/5] Collect test logs with thread safe array lists --- .../openfeature/provider/OctopusContextProviderTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java index 392f186..3bfd7d6 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.logging.Handler; import java.util.logging.LogRecord; import java.util.logging.Logger; @@ -165,7 +166,7 @@ void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() t byte[] initialHash = {0x01, 0x02, 0x03, 0x04}; byte[] updatedHash = {0x01, 0x02, 0x03, 0x05}; - var logMessages = new ArrayList(); + var logMessages = new CopyOnWriteArrayList(); var julLogger = Logger.getLogger(OctopusClient.class.getName()); var handler = new Handler() { @Override public void publish(LogRecord record) { logMessages.add(record.getMessage()); } @@ -236,7 +237,7 @@ void whenAnExceptionIsThrownDuringRefresh_LogsErrorDetails() throws InterruptedE byte[] contentHash = {0x01, 0x02, 0x03, 0x04}; - var logRecords = new ArrayList(); + var logRecords = new CopyOnWriteArrayList(); var julLogger = Logger.getLogger(OctopusClient.class.getName()); var handler = new Handler() { @Override public void publish(LogRecord record) { logRecords.add(record); } From 626dd21d1336c69d2f52f611b2cede278e757c6b Mon Sep 17 00:00:00 2001 From: CaitlynStocker Date: Fri, 29 May 2026 14:27:33 +1000 Subject: [PATCH 5/5] Remove extra layers of error handling --- .../openfeature/provider/OctopusClient.java | 41 +++++++------------ .../provider/OctopusContextProvider.java | 2 +- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index fecebba..59ea702 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -43,7 +43,7 @@ private static String loadProviderVersion() { this.config = config; } - Boolean haveFeatureTogglesChanged(byte[] contentHash) { + Boolean haveFeatureTogglesChanged(byte[] contentHash) throws IOException, InterruptedException { if (contentHash.length == 0) { return true; } @@ -55,18 +55,12 @@ Boolean haveFeatureTogglesChanged(byte[] contentHash) { .header("Authorization", String.format("Bearer %s", config.getClientIdentifier())) .header("X-Octopus-Client", buildOctopusClientHeaderValue()) .build(); - try { - HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); - FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureToggleCheckResponse.class); - return !Arrays.equals(checkResponse.contentHash, contentHash); - } catch (Exception e) { - logger.log(System.Logger.Level.WARNING, String.format("Unable to query Octopus Feature Toggle service. URI: %s", checkURI.toString()), e); - // Use cached toggles - return false; - } + HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); + FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureToggleCheckResponse.class); + return !Arrays.equals(checkResponse.contentHash, contentHash); } - FeatureToggles getFeatureToggleEvaluationManifest() { + FeatureToggles getFeatureToggleEvaluationManifest() throws IOException, InterruptedException { URI manifestURI = getManifestURI(); HttpClient client = HttpClient.newHttpClient(); HttpRequest request = HttpRequest.newBuilder() @@ -75,23 +69,18 @@ FeatureToggles getFeatureToggleEvaluationManifest() { .header("Authorization", String.format("Bearer %s", config.getClientIdentifier())) .header("X-Octopus-Client", buildOctopusClientHeaderValue()) .build(); - try { - HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); - if (httpResponse.statusCode() == StatusCodeNotFound) { - logger.log(System.Logger.Level.WARNING, String.format("Failed to retrieve feature toggles for client identifier %s from %s", config.getClientIdentifier(), manifestURI.toString())); - return null; - } - Optional contentHashHeader = httpResponse.headers().firstValue("ContentHash"); - if (contentHashHeader.isEmpty()) { - logger.log(System.Logger.Level.WARNING, String.format("Feature toggle response from %s did not contain expected ContentHash header", manifestURI.toString())); - return null; - } - var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); - return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); - } catch (Exception e) { - logger.log(System.Logger.Level.WARNING, "Unable to query Octopus Feature Toggle service", e); + HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); + if (httpResponse.statusCode() == StatusCodeNotFound) { + logger.log(System.Logger.Level.WARNING, String.format("Failed to retrieve feature toggles for client identifier %s from %s", config.getClientIdentifier(), manifestURI.toString())); + return null; + } + Optional contentHashHeader = httpResponse.headers().firstValue("ContentHash"); + if (contentHashHeader.isEmpty()) { + logger.log(System.Logger.Level.WARNING, String.format("Feature toggle response from %s did not contain expected ContentHash header", manifestURI.toString())); return null; } + var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); + return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } String buildOctopusClientHeaderValue() { diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java index 0f0ec88..c5c042a 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java @@ -24,7 +24,7 @@ void initialize() { var toggles = client.getFeatureToggleEvaluationManifest(); currentContext = toggles == null ? OctopusContext.empty() : new OctopusContext(toggles); } catch (Exception e) { - logger.log(System.Logger.Level.ERROR, "Failed to retrieve feature toggles during initialization. Falling back to empty context. Default values will be used during evaluated.", e); + logger.log(System.Logger.Level.ERROR, "Failed to retrieve feature manifest during initialization. Falling back to empty context, defaults will be used during evaluation.", e); currentContext = OctopusContext.empty(); }