Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
b38910f
[SDK-1829] chore: add fdv1Fallback field to FDv2 result types
aaron-zeisler Mar 31, 2026
e2bbb3d
[SDK-1829] chore: add fdv1FallbackSynchronizers to mode definitions
aaron-zeisler Mar 31, 2026
425906c
[SDK-1829] chore: detect x-ld-fd-fallback header in FDv2 transport layer
aaron-zeisler Mar 31, 2026
6ba1874
[SDK-1829] chore: add FDv1PollingSynchronizer for fallback polling
aaron-zeisler Mar 31, 2026
c259c4a
[SDK-1829] chore: wire FDv1 fallback into FDv2 data source orchestration
aaron-zeisler Mar 31, 2026
a6860d2
[SDK-1829] refactor: remove unnecessary method overloads from FDv2 types
aaron-zeisler Mar 31, 2026
86d7645
[SDK-1829] refactor: use single nullable FDv1 fallback synchronizer i…
aaron-zeisler Mar 31, 2026
dddc21f
[SDK-1829] feat: detect FDv1 fallback signal during initialization
aaron-zeisler Apr 1, 2026
9af8920
[SDK-1829] fix: forward FDv1 fallback header on 304 Not Modified resp…
aaron-zeisler Apr 1, 2026
3fd905c
[SDK-1829] refactor: inject FeatureFetcher into FDv1PollingSynchronizer
aaron-zeisler Apr 1, 2026
b66ab9e
[SDK-1829] fix: detect FDv1 fallback on initializer result with non-e…
aaron-zeisler Apr 1, 2026
e0eea5d
[SDK-1829] fix: honor FDv1 fallback signal on terminal errors in sync…
aaron-zeisler Apr 1, 2026
3b88fd6
[SDK-1829] refactor: remove ownsFetcher parameter from FDv1PollingSyn…
aaron-zeisler Apr 1, 2026
c89661a
[SDK-1829] refactor: move error classification into fetch callback in…
aaron-zeisler Apr 1, 2026
dde3f8b
[SDK-1829] merging main and resolving conflicts
aaron-zeisler Apr 2, 2026
4af3c19
[SDK-1829] Refine error handling in FDv1PollingSynchronizer
aaron-zeisler Apr 2, 2026
64f3144
[SDK-1829] refactor: consolidate FDv1 fallback check in runInitializers
aaron-zeisler Apr 3, 2026
27ce5a5
[SDK-1829] refactor: internalize FDv1 fallback synchronizer and move …
aaron-zeisler Apr 6, 2026
61d8dca
[SDK-1829] fix: FDv1 fallback respects per-mode poll interval and sur…
aaron-zeisler Apr 6, 2026
7f49606
[SDK-1829] Minor fix to remove redundant else branch
aaron-zeisler Apr 6, 2026
e65f133
[SDK-1829] fix: null-safe default mode in buildModeTable
aaron-zeisler Apr 7, 2026
3afd09a
[SDK-1829] refactor: inline default mode table in FDv2DataSourceBuilder
aaron-zeisler Apr 7, 2026
c6beb5c
[SDK-1829] test: Added capability to support sdk harness test
aaron-zeisler Apr 7, 2026
16b9f8f
[SDK-1829] docs: clarify why empty mode override drops FDv1 fallback
aaron-zeisler Apr 7, 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
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public class TestService extends NanoHTTPD {
"client-prereq-events",
"evaluation-hooks",
"track-hooks",
"client-per-context-summaries"
"client-per-context-summaries",
"client-event-source-http-errors"
};
private static final String MIME_JSON = "application/json";
static final Gson gson = new GsonBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@
import com.launchdarkly.sdk.android.integrations.StreamingSynchronizerBuilder;
import com.launchdarkly.sdk.android.interfaces.ServiceEndpoints;
import com.launchdarkly.sdk.android.subsystems.DataSourceBuildInputs;
import com.launchdarkly.sdk.android.subsystems.DataSourceBuilder;
import com.launchdarkly.sdk.android.subsystems.Initializer;
import com.launchdarkly.sdk.android.subsystems.Synchronizer;
import com.launchdarkly.sdk.internal.http.HttpProperties;

import androidx.annotation.NonNull;

import java.net.URI;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;

/**
* Factory methods for FDv2 data source components used with the
Expand Down Expand Up @@ -102,6 +109,35 @@ public Synchronizer build(DataSourceBuildInputs inputs) {
}
}

static final class FDv1PollingSynchronizerBuilderImpl implements DataSourceBuilder<Synchronizer> {

protected int pollIntervalMillis = LDConfig.DEFAULT_POLL_INTERVAL_MILLIS;

public FDv1PollingSynchronizerBuilderImpl pollIntervalMillis(int pollIntervalMillis) {
this.pollIntervalMillis = pollIntervalMillis <= LDConfig.DEFAULT_POLL_INTERVAL_MILLIS ?
LDConfig.DEFAULT_POLL_INTERVAL_MILLIS : pollIntervalMillis;
return this;
}

@Override
public Synchronizer build(DataSourceBuildInputs inputs) {
FeatureFetcher fetcher = new HttpFeatureFlagFetcher(
inputs.getServiceEndpoints().getPollingBaseUri(),
inputs.isEvaluationReasons(),
inputs.getHttp().isUseReport(),
LDUtil.makeHttpProperties(inputs.getHttp()),
inputs.getCacheDir(),
inputs.getBaseLogger()
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FDv1 fallback polling uses wrong endpoint path

High Severity

The FDv1PollingSynchronizerBuilderImpl creates an HttpFeatureFlagFetcher using inputs.getServiceEndpoints().getPollingBaseUri() as the polling URI. However, getPollingBaseUri() returns the FDv2 polling base URI. The FDv1 fallback synchronizer needs to reach the legacy FDv1 /sdk/evalx endpoints, but HttpFeatureFlagFetcher appends FDv1 paths (StandardEndpoints.POLLING_REQUEST_GET_BASE_PATH) to whatever base URI it receives. If the FDv2 polling base URI differs from the FDv1 polling base URI (which is the case when custom service endpoints are configured separately for FDv2), the requests will be sent to the wrong host or path.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 16b9f8f. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bugbot is confused between the base URI and the full path. The pollingBaseUri is really just the base url, and it's the same between FDv1 and FDv2. Then the correct path is added to the baseUri when the fetch() function is called.

return new FDv1PollingSynchronizer(
inputs.getEvaluationContext(), fetcher,
inputs.getSharedExecutor(), 0,
pollIntervalMillis,
inputs.getBaseLogger()
);
}
}

/**
* Returns a builder for a polling initializer.
* <p>
Expand Down Expand Up @@ -141,6 +177,63 @@ public static StreamingSynchronizerBuilder streamingSynchronizer() {
return new StreamingSynchronizerBuilderImpl();
}

/**
* Produces the default mode table used by {@link DataSystemBuilder#buildModeTable}.
* Defined here (rather than in {@code DataSystemBuilder}) because the FDv1 fallback
* synchronizer references package-private types that are not visible from the
* {@code integrations} package.
* <p>
* This method is public only for cross-package access within the SDK; it is not
* intended for use by application code.
*/
@NonNull
public static Map<ConnectionMode, ModeDefinition> makeDefaultModeTable() {
DataSourceBuilder<Initializer> pollingInitializer = pollingInitializer();
DataSourceBuilder<Synchronizer> pollingSynchronizer = pollingSynchronizer();
DataSourceBuilder<Synchronizer> streamingSynchronizer = streamingSynchronizer();
DataSourceBuilder<Synchronizer> backgroundPollingSynchronizer =
pollingSynchronizer()
.pollIntervalMillis(LDConfig.DEFAULT_BACKGROUND_POLL_INTERVAL_MILLIS);
DataSourceBuilder<Synchronizer> fdv1FallbackPollingSynchronizerForeground =
new FDv1PollingSynchronizerBuilderImpl();

DataSourceBuilder<Synchronizer> fdv1FallbackPollingSynchronizerBackground =
new FDv1PollingSynchronizerBuilderImpl().pollIntervalMillis(LDConfig.DEFAULT_BACKGROUND_POLL_INTERVAL_MILLIS);

Map<ConnectionMode, ModeDefinition> table = new LinkedHashMap<>();
table.put(ConnectionMode.STREAMING, new ModeDefinition(
// TODO: cacheInitializer — add once implemented
Arrays.asList(/* cacheInitializer, */ pollingInitializer),
Arrays.asList(streamingSynchronizer, pollingSynchronizer),
fdv1FallbackPollingSynchronizerForeground
));
table.put(ConnectionMode.POLLING, new ModeDefinition(
// TODO: Arrays.asList(cacheInitializer) — add once implemented
Collections.<DataSourceBuilder<Initializer>>emptyList(),
Collections.singletonList(pollingSynchronizer),
fdv1FallbackPollingSynchronizerForeground
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shared FDv1 fallback builder instance across STREAMING and POLLING modes

Low Severity

The same fdv1FallbackPollingSynchronizerForeground builder instance is shared between the STREAMING and POLLING mode definitions. Since FDv1PollingSynchronizerBuilderImpl is mutable (it has a settable pollIntervalMillis), if any code path ever mutates this shared builder after table construction, it would affect both modes. While currently safe because the builder is not further modified, sharing a mutable builder across multiple ModeDefinition entries is fragile.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3afd09a. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is also by design. I think the alternative is to create two separate FDv1PollingSycnhronizer builders (and i believe only their polling durations would be different).

));
table.put(ConnectionMode.OFFLINE, new ModeDefinition(
// TODO: Arrays.asList(cacheInitializer) — add once implemented
Collections.<DataSourceBuilder<Initializer>>emptyList(),
Collections.<DataSourceBuilder<Synchronizer>>emptyList(),
null
));
table.put(ConnectionMode.ONE_SHOT, new ModeDefinition(
// TODO: cacheInitializer and streamingInitializer — add once implemented
Arrays.asList(/* cacheInitializer, */ pollingInitializer /*, streamingInitializer, */),
Collections.<DataSourceBuilder<Synchronizer>>emptyList(),
null
));
table.put(ConnectionMode.BACKGROUND, new ModeDefinition(
// TODO: Arrays.asList(cacheInitializer) — add once implemented
Collections.<DataSourceBuilder<Initializer>>emptyList(),
Collections.singletonList(backgroundPollingSynchronizer),
fdv1FallbackPollingSynchronizerBackground
));
return table;
}

/**
* Returns a builder for configuring automatic connection mode switching in response to
* platform events (foreground/background and network availability).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,21 @@ public void onResponse(@NonNull Call call, @NonNull Response response) {
return future;
}

private static boolean isFdv1Fallback(@NonNull Response response) {
String value = response.header(HeaderConstants.FDV1_FALLBACK_HEADER);
return value != null && value.equalsIgnoreCase("true");
}

private void handleResponse(
@NonNull Response response,
@NonNull LDAwaitFuture<FDv2PayloadResponse> future) {
try {
int code = response.code();
boolean fdv1Fallback = isFdv1Fallback(response);

if (code == 304) {
logger.debug("FDv2 polling: 304 Not Modified");
future.set(FDv2PayloadResponse.notModified());
future.set(FDv2PayloadResponse.notModified(fdv1Fallback));
return;
}

Expand All @@ -197,7 +203,7 @@ private void handleResponse(
} else {
logger.warn("Polling request failed with HTTP {}", code);
}
future.set(FDv2PayloadResponse.failure(code));
future.set(FDv2PayloadResponse.failure(code, fdv1Fallback));
return;
}

Expand All @@ -216,7 +222,7 @@ private void handleResponse(
String bodyStr = body.string();
logger.debug("Polling response received");
List<FDv2Event> events = FDv2Event.parseEventsArray(bodyStr);
future.set(FDv2PayloadResponse.success(events, code));
future.set(FDv2PayloadResponse.success(events, code, fdv1Fallback));

} catch (Exception e) {
future.setException(e);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
package com.launchdarkly.sdk.android;

import androidx.annotation.NonNull;

import com.launchdarkly.logging.LDLogger;
import com.launchdarkly.sdk.LDContext;
import com.launchdarkly.sdk.android.DataModel.Flag;
import com.launchdarkly.sdk.android.subsystems.Callback;
import com.launchdarkly.sdk.android.subsystems.FDv2SourceResult;
import com.launchdarkly.sdk.android.subsystems.Synchronizer;
import com.launchdarkly.sdk.fdv2.ChangeSet;
import com.launchdarkly.sdk.fdv2.ChangeSetType;
import com.launchdarkly.sdk.fdv2.Selector;
import com.launchdarkly.sdk.fdv2.SourceResultType;
import com.launchdarkly.sdk.fdv2.SourceSignal;
import com.launchdarkly.sdk.json.SerializationException;

import java.io.IOException;
import java.util.Map;
import java.util.concurrent.Future;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

/**
* FDv1 polling synchronizer used as a fallback when the server signals that FDv2 endpoints
* are unavailable via the {@code x-ld-fd-fallback} response header.
* <p>
* Delegates the actual HTTP fetch to a {@link FeatureFetcher} (the same transport used by the
* production FDv1 polling data source) and converts the response into {@link FDv2SourceResult}
* objects so it can be used as a drop-in synchronizer within the FDv2 data source pipeline.
*/
final class FDv1PollingSynchronizer implements Synchronizer {

private final LDContext evaluationContext;
private final FeatureFetcher fetcher;
private final LDLogger logger;

private final LDAsyncQueue<FDv2SourceResult> resultQueue = new LDAsyncQueue<>();
private final LDAwaitFuture<FDv2SourceResult> shutdownFuture = new LDAwaitFuture<>();

private volatile ScheduledFuture<?> scheduledTask;
private final Object taskLock = new Object();

/**
* @param evaluationContext the context to evaluate flags for
* @param fetcher the HTTP transport for FDv1 polling requests
* @param executor scheduler for recurring poll tasks
* @param initialDelayMillis delay before the first poll in milliseconds
* @param pollIntervalMillis delay between the end of one poll and the start of the next
* @param logger logger
*/
FDv1PollingSynchronizer(
@NonNull LDContext evaluationContext,
@NonNull FeatureFetcher fetcher,
@NonNull ScheduledExecutorService executor,
long initialDelayMillis,
long pollIntervalMillis,
@NonNull LDLogger logger) {
this.evaluationContext = evaluationContext;
this.fetcher = fetcher;
this.logger = logger;

synchronized (taskLock) {
scheduledTask = executor.scheduleWithFixedDelay(
this::pollAndEnqueue,
initialDelayMillis,
pollIntervalMillis,
TimeUnit.MILLISECONDS);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shared single-thread executor risks deadlock with blocking poll

Medium Severity

FDv1PollingSynchronizer schedules its recurring poll task on inputs.getSharedExecutor() — the same 4-thread pool used by FDv2DataSource's main loop, condition timers, and other synchronizers. The doPoll() method blocks the executor thread via resultFuture.get() while waiting for the async HTTP callback. Combined with the main data source loop also occupying a thread on the same pool, this could lead to thread starvation or deadlock under load, especially during the transition period when both FDv2 and FDv1 components are active.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this problem is theoretical only. In practice, when the fallback synchronizer is in use, all the other initializers and synchronizers will be closed. In other words, only one synchronizer runs at a time.

Regarding the statement "... especially during the transition period when both FDv2 and FDv1 components are active": The old synchronizer is fully closed before the new one is even built. There's no window where both are running simultaneously. I'm not sure what "transition period" is referring to.

Copy link
Copy Markdown
Contributor

@tanderson-ld tanderson-ld Apr 3, 2026

Choose a reason for hiding this comment

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

Maybe see if you can figure out why Cursor bugbot thinks and other synchronizers is possible at the same time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I asked Cursor to analyze this statement a few different ways. In each case, it seems like Cursor generally knew how the shared executor worked because of the comments in that class's definition. But when it made this comment on my PR, it hadn't taken one step further to understand exactly how many threads were being used, and how the threads are being juggled.

After writing this, it seems like adding a little bit more to the comments may help in the future.

}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FDv1 synchronizer starts polling immediately in constructor

Medium Severity

FDv1PollingSynchronizer starts its scheduled polling task in the constructor with initialDelayMillis=0. The factory in FDv1PollingSynchronizerBuilderImpl.build() passes 0 for initialDelayMillis. Since SourceManager.getNextAvailableSynchronizerAndSetActive() calls factory.build() which constructs this synchronizer, polling begins immediately upon construction — potentially before the caller has called next() to consume results. If the first poll completes very quickly, the result goes into resultQueue which is fine, but if a terminal error occurs before next() is ever called, shutdownFuture is set and the scheduled task cancelled. Subsequent next() calls will return the terminal result, which is correct. However, this eager-start pattern means the synchronizer fires off network requests before the FDv2DataSource run loop is ready to consume them, which could cause unexpected behavior if the executor thread pool is constrained.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 16b9f8f. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was our design choice. The FDv2 polling synchronizer behaves the same way.


private void pollAndEnqueue() {
try {
FDv2SourceResult result = doPoll();

if (result.getResultType() == SourceResultType.STATUS) {
FDv2SourceResult.Status status = result.getStatus();
if (status != null && status.getState() == SourceSignal.TERMINAL_ERROR) {
synchronized (taskLock) {
if (scheduledTask != null) {
scheduledTask.cancel(false);
scheduledTask = null;
}
}
shutdownFuture.set(result);
return;
}
}

resultQueue.put(result);
} catch (RuntimeException e) {
LDUtil.logExceptionAtErrorLevel(logger, e, "Unexpected exception in FDv1 polling synchronizer task");
resultQueue.put(FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(e), false));
}
}

/**
* Fetches flags via FDv1 polling and converts the result to an {@link FDv2SourceResult}.
* <p>
* All result/error processing happens inside the {@link Callback} so the future carries a
* fully-formed {@link FDv2SourceResult}. This keeps application-level error classification
* (e.g. {@link LDInvalidResponseCodeFailure}) at the callback layer rather than unwrapping
* it from a {@link java.util.concurrent.Future#get()} exception.
*/
private FDv2SourceResult doPoll() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code review notes:

  • Can we safely refactor the production FDv1 code to reuse this entire doPoll() function?
  • Similarly, can we reuse the existing code on lines 87-101?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I haven't addressed this yet

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In my latest push, I've refactored this new FDv1PollingSynchronizer by injecting a FeatureFetcher object. That's the same helper class that the existing PollingDataSource uses 👍

LDAwaitFuture<FDv2SourceResult> resultFuture = new LDAwaitFuture<>();

fetcher.fetch(evaluationContext, new Callback<String>() {
@Override
public void onSuccess(String json) {
try {
logger.debug("FDv1 fallback polling response received");
EnvironmentData envData = EnvironmentData.fromJson(json);
Map<String, Flag> flags = envData.getAll();

ChangeSet<Map<String, Flag>> changeSet = new ChangeSet<>(
ChangeSetType.Full,
Selector.EMPTY,
flags,
null,
true);
resultFuture.set(FDv2SourceResult.changeSet(changeSet, false));
} catch (SerializationException e) {
LDUtil.logExceptionAtErrorLevel(logger, e, "FDv1 fallback polling failed to parse response");
LDFailure failure = new LDFailure(
"FDv1 fallback: invalid JSON response", e, LDFailure.FailureType.INVALID_RESPONSE_BODY);
resultFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(failure), false));
}
}

@Override
public void onError(Throwable e) {
if (e instanceof LDInvalidResponseCodeFailure) {
int code = ((LDInvalidResponseCodeFailure) e).getResponseCode();
if (code == 400) {
logger.error("Received 400 response when fetching flag values. " +
"Please check recommended R8 and/or ProGuard settings");
}
boolean recoverable = LDUtil.isHttpErrorRecoverable(code);
logger.warn("FDv1 fallback polling failed with HTTP {}", code);
LDFailure failure = new LDInvalidResponseCodeFailure(
"FDv1 fallback polling request failed", e, code, recoverable);
if (!recoverable) {
resultFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.terminalError(failure), false));
} else {
resultFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(failure), false));
}
} else if (e instanceof IOException) {
LDUtil.logExceptionAtErrorLevel(logger, e, "FDv1 fallback polling failed with network error");
resultFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(e), false));
} else {
LDUtil.logExceptionAtErrorLevel(logger, e, "FDv1 fallback polling failed");
resultFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(e), false));
}
}
});

try {
return resultFuture.get();
} catch (InterruptedException e) {
return FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(e), false);
} catch (Exception e) {
// Should not happen — all callback paths call set(), never setException()
Throwable cause = e.getCause() != null ? e.getCause() : e;
LDUtil.logExceptionAtErrorLevel(logger, cause, "FDv1 fallback polling failed unexpectedly");
return FDv2SourceResult.status(FDv2SourceResult.Status.interrupted(cause), false);
}
}

@Override
@NonNull
public Future<FDv2SourceResult> next() {
return LDFutures.anyOf(shutdownFuture, resultQueue.take());
}

@Override
public void close() {
synchronized (taskLock) {
if (scheduledTask != null) {
scheduledTask.cancel(false);
scheduledTask = null;
}
}
shutdownFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.shutdown(), false));
try {
fetcher.close();
} catch (IOException ignored) {
}
}
}
Loading
Loading