Create a new feature-flag-controller

[vthomas13]

Objectives

The FeatureFlagController will be a specialized extension of BaseController with state management, caching, and feature flag fetching capabilities. It will use ClientConfigApi to retrieve feature flags from LaunchDarkly, applying caching to optimize API usage and ensuring user privacy through configurable settings.

1. Centralized Access: Expose feature flags consistently across both the Extension and Mobile clients.
2. Caching: Reduce redundant API calls by caching flag values for a short period, enhancing performance and reliability.
3. Fallback Logic: Ensure functionality in cases where the remote server is unreachable, feature flag values should fall back to those stored locally or, if no cache exists, to default values hardcoded in the codebase.

High-Level Architecture

The core controller will:

• Be published as @metamask/feature-flag-controller, detailed configuration for package.json can refer to packages/address-book-controller/package.json
• Request feature flag values from ClientConfigApi

curl --location 'https://client-config.api.cx.metamask.io/v1/flags?client=extension&distribution=main&environment=dev' \
--header 'Accept: application/json'

• Apply caching to reduce the load on ClientConfigApi.
  Provide a fallback mechanism, either using cached values or default values in case of API unavailability, or a request is made during the updating, or no cache exists.
• Expose methods for retrieving feature flag values to both the Extension and Mobile clients.

Implementation Details

Controller guideline:
https://github.com/MetaMask/core/blob/main/docs/writing-controllers.md
Best practice:
https://github.com/MetaMask/core/tree/0fc0396cbcaae95ea5c8bf3f92fef18adc496283/examples/example-controllers
Service guidance:
https://github.com/MetaMask/decisions/blob/main/decisions/core/0002-external-api-integrations.md

1. Define Initial State and Metadata

Set up the initial state and metadata for the FeatureFlagController to hold feature flags and cache details. This state will persist between sessions for reliability in offline scenarios.

type FeatureFlagControllerState = {
  flags: Record<string, boolean>; // Cached feature flag values
  cacheTimestamp: number;          // Timestamp to validate cache freshness
};

2. Implement Caching Mechanism
To reduce API calls, we’ll implement a cache with a validity period. Use cacheTimestamp to track when the last fetch occurred, and check this before fetching again. Also allow the caller to specify how frequently the API should be called by adding an optional fetchInterval parameter.

// Define the default cache duration (24 hours in milliseconds)
const DEFAULT_CACHE_DURATION = 24 * 60 * 60 * 1000; // 1 day TBD

// Method to check if cache is still valid
private isCacheValid(fetchInterval: number = DEFAULT_CACHE_DURATION): boolean {
  return (Date.now() - this.state.cacheTimestamp) < fetchInterval;
}

3. Fetch Feature Flags from API in a service
The feature-flag-service will fetch feature flags from ClientConfigApi. It should handle cases with and without a profileId for user-specific flags. Implement error handling to manage API failures and cache fallback if needed.

// Define accepted values for client, distribution, and environment
type ClientType = 'extension' | 'mobile';
type DistributionType = 'main' | 'flask';
type EnvironmentType = 'prod' | 'rc' | 'dev';

// Method to fetch flags from the API with mandatory parameters and optional profileId
private async _fetchFlagsFromApi(
  client: ClientType,
  distribution: DistributionType,
  environment: EnvironmentType,
): Promise<Record<string, boolean>> {
  const url = `https://client-config.api.cx.metamask.io/v1/flags?client=${client}&distribution=${distribution}&environment=${environment}${profileId ? `&user=${profileId}` : ''}`;
  
  try {
    const response = await handleFetch(url);
    if (!response.ok) throw new Error('Failed to fetch flags');
    return await response.json();
  } catch (error) {
    console.error('Feature flag API request failed:', error);

  // Check if there's cached data to include in the response
  const cachedData = getCachedData(url); // Assuming this function retrieves the cached data
  const cacheTimestamp = cachedData ? cachedData.timestamp : null;

  // Return a structured error object to the client
  return {
    error: true,
    message: error.message,            // The error message
    statusCode: response ? response.status : null, // HTTP status if available
    statusText: response ? response.statusText : null, // HTTP status text if available
    cachedData: cachedData || {},      // Return cached data if available, or an empty object
    cacheTimestamp: cacheTimestamp,    // Include the timestamp of cached data if available
  }
}

and use in controller:

// Public method to retrieve all flags with caching, passing mandatory params
public async getFeatureFlags(
  client: ClientType,
  distribution: DistributionType,
  environment: EnvironmentType,
  fetchInterval?: number = DEFAULT_CACHE_DURATION,
): Promise<Record<string, boolean>> {
  // Use cached flags if the cache is still valid
  if (this.isCacheValid(fetchInterval)) {
    return this.state.flags;
  }

  // Attempt to fetch new flags from the API
  const flags = await this.fetchFlagsFromApi(client, distribution, environment);

  // Check if we got a valid response from the API
  if (Object.keys(flags).length > 0) {
    this.updateCache(flags); // Update cache if we received flags from the API
    return flags;
  }

  // Fallback to cached flags if API failed and we have cache available
  if (Object.keys(this.state.flags).length > 0) {
    return this.state.flags;
  }

  // Fallback to empty object if both API and cache are unavailable
  // Client can determine the value if the flags are empty
  return {};
}


// Update cache with new flags and timestamp
private updateCache(flags: Record<string, boolean>) {
  this.update((state) => {
    state.flags = flags;
    state.cacheTimestamp = Date.now();
  });
}

4. Apply Fallback Logic
In case of an API failure or offline status, fallback to either cached values (if available) or default hardcoded values.

**5. Handle Simultaneous Requests with In-Progress Tracking **
The feature-flag-controller will use in-progress flags (e.g., refer to #inProgressHotlistUpdate in phishing-controller) to ensure only one update of each type occurs at any time, waiting for completion if an update is already ongoing.
This prevents redundant requests and allows the controller to be more resilient to concurrent updates, particularly helpful in a multithreaded or asynchronous environment.

public async getFeatureFlags(clientType, distributionType, environmentType): Promise<Record<string, X>> {
  if (this.isCacheValid()) {
    return this.state.flags;
  }

  if (this.#inProgressFlagUpdate) {
    await this.#inProgressFlagUpdate;
  }

  try {
    this.#inProgressFlagUpdate = this.fetchFlagsFromApi(clientType, distributionType, environmentType);
    const flags = await this.#inProgressFlagUpdate;

    if (Object.keys(flags).length > 0) {
      this.updateCache(flags);
      return flags;
    }
  } finally {
    this.#inProgressFlagUpdate = undefined;
  }

  return this.state.flags;
}

6. Extend with Messaging and Registering Handlers
Set up message handlers to manage interactions and handle actions (e.g., requesting flag updates).

#registerMessageHandlers(): void {
  this.messagingSystem.registerActionHandler(
    `${controllerName}:maybeUpdateState` as const,
    this.maybeUpdateState.bind(this),
  );
}

Scenario

No response

Design

No response

Technical Details

No response

Threat Modeling Framework

No response

Acceptance Criteria

1. A new controller feature-controller is added to core and published to be used, following the recommended pattern: https://github.com/MetaMask/core/tree/0fc0396cbcaae95ea5c8bf3f92fef18adc496283/examples/example-controllers
2. when a consumer requests ALL feature flags from controller, controller will make request to clientConfigAPI based on client, distribution, environment and profileId (optional), and fetchInterval (optional, default 30 days)
3. when clientConfigAPI is down, controller will return the cached value from previous recent fetch
4. when there is no cached value stored, controller will return {} and client will decide which value as default value
5. unit tests will be included in this ticket

Stakeholder review needed before the work gets merged

• Engineering (needed in most cases)
• Design
• Product
• QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
• Security
• Legal
• Marketing
• Management (please specify)
• Other (please specify)

References

No response

[joaoloureirop]

It should be packages/feature-flag-controller/package.json for the detailed configuration path, correct?

I have two questions / suggestions:

1. For when the clientConfigAPI is down, fetch will rely on a retry logic? Or the default fetch (UI load for extension, cold start for Mobile) is enough?
2. The controller state should be compatible with all flag types (boolean, number, string, json)

[DDDDDanica]

hey @joaoloureirop thanks for reviewing,

It should be packages/feature-flag-controller/package.json for the detailed configuration path, correct?

Yes indeed, the one is the expected path

For when the clientConfigAPI is down, fetch will rely on a retry logic? Or the default fetch (UI load for extension, cold start for Mobile) is enough?

If API is down, the controller actually returns a cache value from previous fetch; further more, if there's no cache available in the storage, an empty object is returned, and we can have customization in extension of mobile to decide which is the default value we want to return.

The controller state should be compatible with all flag types (boolean, number, string, json)

definitely ! The code snippet above will act as an example for some critical logics, when implementing, we have some best practice to follow, include strict types

[joaoloureirop]

Thanks for clarifying @DDDDDanica !

If API is down, the controller actually returns a cache value from previous fetch; further more, if there's no cache available in the storage, an empty object is returned, and we can have customization in extension of mobile to decide which is the default value we want to return.

Ah yes, it is up to the client (mobile, extension) to handle the empty cache return.
But how will the controller behave with non-2xx HTTP responses?

[gauthierpetetin]

Thanks for this technical refinement of feature flags. Here are some comments:

Use profileId to determine unique user-based flag values.

=> profileId is not passed to ClientConfigApi anymore in the new version of the ADR, it's worth being re-read if you've only seen the old version of it.

const DEFAULT_CACHE_DURATION = 30 * 24 * 60 * 60 * 1000; // 30 days

=> The default cache duration should be 1 hour rather than 30 days because if there’s a bug in a feature that requires disabling, we want it to be disabled quickly.

[DDDDDanica]

Hey @gauthierpetetin thanks for reviewing:

profileId is not passed to ClientConfigApi anymore in the https://github.com/MetaMask/decisions/pull/43, it's worth being re-read if you've only seen the old version of it.

Good catch, gonna remove this feature

The default cache duration should be 1 hour rather than 30 days because if there’s a bug in a feature that requires disabling, we want it to be disabled quickly.

Updated to daily update, to be decided until we get an answer of capacity of config api

[DDDDDanica]

@joaoloureirop thanks for pointing out the non-2xx response, we will return an error object as well, I'll add these to the refinement let me know your thoughts

• it will return cached data if available, rather than failing outright.
• the controller will log the error and return a structured error object to the client, including timestamp of cached data,
• If there is no cache available and a non-2xx response is returned, the controller should return an empty object or a predefined default object to allow customization by the client

[joaoloureirop]

@joaoloureirop thanks for pointing out the non-2xx response, we will return an error object as well, I'll add these to the refinement let me know your thoughts

• it will return cached data if available, rather than failing outright.
• the controller will log the error and return a structured error object to the client, including timestamp of cached data,
• If there is no cache available and a non-2xx response is returned, the controller should return an empty object or a predefined default object to allow customization by the client

Sorry, I wasn't very clear about the question about failed fetches.

I meant if the controller would handle failed requests with a retry logic.
In scenarios where the connection is unstable, HTTP requests have a higher chance of failing.
Without a retry logic, the controller will only request again on a specific stage (UI load on extension, app cold start on mobile).
With a retry logic, the controller would keep trying to fetch feature flags until it gets a successful response

[DDDDDanica]

@joaoloureirop That's a very valid point, I will refer the retry system applied to codify service here as

Construct a policy that will retry each update, and halt further updates for a certain period after too many consecutive failures.