Skip to content

fix: Simplify refresh logic on a failed fetch #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
caitlynstocker merged 5 commits into from
May 29, 2026
+169 −40
Merged

fix: Simplify refresh logic on a failed fetch #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3502432
Remove 5 second retry from context provider
caitlynstocker May 28, 2026
8a7d553
Add tests
caitlynstocker May 28, 2026
4ea2f9b
Hide scary logging from refresh failures in tests
caitlynstocker May 28, 2026
111a4be
Collect test logs with thread safe array lists
caitlynstocker May 29, 2026
626dd21
Remove extra layers of error handling
caitlynstocker May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 15 additions & 26 deletions src/main/java/com/octopus/openfeature/provider/OctopusClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -55,18 +55,12 @@ Boolean haveFeatureTogglesChanged(byte[] contentHash) {
.header("Authorization", String.format("Bearer %s", config.getClientIdentifier()))
.header("X-Octopus-Client", buildOctopusClientHeaderValue())
.build();
try {
HttpResponse<String> 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<String> httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString());
FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureToggleCheckResponse.class);
return !Arrays.equals(checkResponse.contentHash, contentHash);
}
Copy link
Copy Markdown
Contributor Author

@caitlynstocker caitlynstocker May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removes error handling for consistency with the HaveFeaturesChanged function in the .NET provider. Errors here will now bubble up to be logged in the catch blocks of the OctopusContextProvider's refresh() function.


FeatureToggles getFeatureToggleEvaluationManifest() {
FeatureToggles getFeatureToggleEvaluationManifest() throws IOException, InterruptedException {
URI manifestURI = getManifestURI();
HttpClient client = HttpClient.newHttpClient();
HttpRequest request = HttpRequest.newBuilder()
Expand All @@ -75,23 +69,18 @@ FeatureToggles getFeatureToggleEvaluationManifest() {
.header("Authorization", String.format("Bearer %s", config.getClientIdentifier()))
.header("X-Octopus-Client", buildOctopusClientHeaderValue())
.build();
try {
HttpResponse<String> 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<String> 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<List<FeatureToggleEvaluation>>() {});
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<String> 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<String> 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<List<FeatureToggleEvaluation>>() {});
return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get()));
Copy link
Copy Markdown
Contributor Author

@caitlynstocker caitlynstocker May 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removes error handling for consistency with the GetFeatureToggleEvaluationManifest function in the .NET provider. Errors here will now bubble up to be logged in the catch blocks of the OctopusContextProvider's initialize() and refresh() functions.

}

String buildOctopusClientHeaderValue() {
Expand Down
17 changes: 3 additions & 14 deletions src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
package com.octopus.openfeature.provider;

import java.time.Duration;

class OctopusContextProvider {
private final OctopusConfiguration config;
private final OctopusClient client;
private boolean initialized = false;
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;
Expand All @@ -27,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();
}

Expand All @@ -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();
Expand All @@ -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);
}
}
}
Expand Down
151 changes: 151 additions & 0 deletions src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -105,6 +106,9 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws
try {
provider.initialize();

// Stop warnings about refresh failures from reaching the test output
julLogger.setUseParentHandlers(false);

Copy link
Copy Markdown
Contributor Author

@caitlynstocker caitlynstocker May 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QUESTION: The tests involving refresh failures were outputting a lot of scary looking logs - which is expected as we're throwing an error every time a refresh fails. I've added these lines to keep the test output clean, however this could potentially hide other more useful logs. What is the better option here between hiding logs and showing too many?

Here is the combined test output before adding these limitations to the test loggers.

SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:02 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:02 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:02 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:02 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:02 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:02 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
... this x about 10

java.lang.RuntimeException: Oops! Simulated refresh error
	at com.octopus.openfeature.provider.OctopusContextProviderTests$ThrowsOnRefreshClient.haveFeatureTogglesChanged(OctopusContextProviderTests.java:225)

	at com.octopus.openfeature.provider.OctopusContextProvider.refresh(OctopusContextProvider.java:48)
	at java.base/java.lang.Thread.run(Thread.java:1474)

May 28, 2026 2:22:08 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
java.lang.RuntimeException: Oops! Simulated refresh error
	at com.octopus.openfeature.provider.OctopusContextProviderTests$ThrowsOnRefreshClient.haveFeatureTogglesChanged(OctopusContextProviderTests.java:225)

	at com.octopus.openfeature.provider.OctopusContextProvider.refresh(OctopusContextProvider.java:48)
	at java.base/java.lang.Thread.run(Thread.java:1474)
... this x 3

May 28, 2026 2:22:09 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:09 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:09 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
May 28, 2026 2:22:09 PM com.octopus.openfeature.provider.OctopusContextProvider refresh
SEVERE: Failed to retrieve updated feature manifest. Retaining existing context which may be stale.
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.66 s -- in com.octopus.openfeature.provider.OctopusContextProviderTests

Results:

Tests run: 5, Failures: 0, Errors: 0, Skipped: 0

------------------------------------------------------------------------
BUILD SUCCESS
------------------------------------------------------------------------
Total time:  20.460 s
Finished at: 2026-05-28T14:22:14+10:00
------------------------------------------------------------------------

Copy link
Copy Markdown
Contributor

@liamhughes liamhughes May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly either way, but given we are adding the logger to assert against, this test output is a side effect and it is reasonable to suppress it.

// Simulate a failed fetch
client.changeToggles(null);

Expand All @@ -118,6 +122,153 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws

} finally {
julLogger.removeHandler(handler);
julLogger.setUseParentHandlers(true);
provider.shutdown();
}
}

Copy link
Copy Markdown
Contributor Author

@caitlynstocker caitlynstocker May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests below have been translated straight across from my .NET PR for the same ticket.

@Test
void whenInitialFetchReturnsNothing_AndRefreshSucceeds_ContextIsPopulated() throws InterruptedException {

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);
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 {
julLogger.setUseParentHandlers(true);
provider.shutdown();
}
}

@Test
void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() throws InterruptedException {

byte[] initialHash = {0x01, 0x02, 0x03, 0x04};
byte[] updatedHash = {0x01, 0x02, 0x03, 0x05};

var logMessages = new CopyOnWriteArrayList<String>();
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();

// 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);
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);
julLogger.setUseParentHandlers(true);
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 CopyOnWriteArrayList<LogRecord>();
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);
// 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(
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);
julLogger.setUseParentHandlers(true);
provider.shutdown();
}
}
Expand Down
Loading