Skip to content

Commit 9011c2a

Browse files
joker23joker23
authored
feat: wire registerDebugOverrides through client common (#1368)
This PR will rewire the registerDebugOverrides callback to enable the LDDebugOverride plugins to all client side sdks. The wiring will require setting of an internal options so this feature is only enabled for electron, brower, and react sdks for now. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches shared client initialization (`LDClientImpl` constructor) to invoke plugin debug registration earlier, which could affect plugin behavior/order during startup across SDKs. Changes are scoped to optional internal wiring and include unit coverage, but impact spans multiple client implementations. > > **Overview** > Adds a new internal `LDClientInternalOptions.registerDebugOverrides` callback and invokes it during `LDClientImpl` construction when `FlagManager.getDebugOverride()` is available, enabling plugins to receive debug override controls as part of client initialization. > > Updates Browser and Electron clients to pass `registerDebugOverrides` that calls `safeRegisterDebugOverridePlugins`, and removes the prior Browser-side post-plugin-registration debug override wiring. > > Extracts `LDDebugOverride` into a dedicated API type (`api/LDDebugOverride`) and updates imports/exports accordingly, with new unit tests covering `safeRegisterDebugOverridePlugins` behavior (skipping missing hooks and continuing after exceptions). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5252b49. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 01dbadb commit 9011c2a

10 files changed

Lines changed: 174 additions & 54 deletions

File tree

‎packages/sdk/browser/src/BrowserClient.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ class BrowserClientImpl extends LDClientImpl {
149149
),
150150
getImplementationHooks: (environmentMetadata: LDPluginEnvironmentMetadata) =>
151151
internal.safeGetHooks(logger, environmentMetadata, validatedBrowserOptions.plugins),
152+
registerDebugOverrides: (debugOverride) =>
153+
safeRegisterDebugOverridePlugins(logger, debugOverride, validatedBrowserOptions.plugins),
152154
credentialType: 'clientSideId',
153155
requiresStart: true,
154156
initialContext,
@@ -223,11 +225,6 @@ class BrowserClientImpl extends LDClientImpl {
223225
client,
224226
this._plugins || [],
225227
);
226-
227-
const override = this.getDebugOverrides();
228-
if (override) {
229-
safeRegisterDebugOverridePlugins(this.logger, override, this._plugins || []);
230-
}
231228
}
232229

233230
override async identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void> {

‎packages/sdk/electron/src/ElectronClient.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
LDWaitForInitializationOptions,
2424
LDWaitForInitializationResult,
2525
mobileFdv1Endpoints,
26+
safeRegisterDebugOverridePlugins,
2627
} from '@launchdarkly/js-client-sdk-common';
2728

2829
import ElectronDataManager from './ElectronDataManager';
@@ -88,6 +89,8 @@ export class ElectronClient extends LDClientImpl {
8889
highTimeoutThreshold: 15,
8990
getImplementationHooks: (_environmentMetadata: LDPluginEnvironmentMetadata) =>
9091
internal.safeGetHooks(logger, _environmentMetadata, validatedElectronOptions.plugins),
92+
registerDebugOverrides: (debugOverride) =>
93+
safeRegisterDebugOverridePlugins(logger, debugOverride, validatedElectronOptions.plugins),
9194
credentialType: useClientSideId ? 'clientSideId' : 'mobileKey',
9295
requiresStart: true,
9396
initialContext,

‎packages/shared/sdk-client/__tests__/plugins/safeRegisterDebugOverridePlugins.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { LDLogger } from '@launchdarkly/js-sdk-common';
2+
3+
import { LDPluginBase } from '../../src/api';
4+
import { LDDebugOverride } from '../../src/api/LDDebugOverride';
5+
import { safeRegisterDebugOverridePlugins } from '../../src/plugins/safeRegisterDebugOverridePlugins';
6+
7+
function createMockLogger(): LDLogger {
8+
return {
9+
error: jest.fn(),
10+
warn: jest.fn(),
11+
info: jest.fn(),
12+
debug: jest.fn(),
13+
};
14+
}
15+
16+
function createMockDebugOverride(): LDDebugOverride {
17+
return {
18+
setOverride: jest.fn(),
19+
removeOverride: jest.fn(),
20+
clearAllOverrides: jest.fn(),
21+
getAllOverrides: jest.fn().mockReturnValue({}),
22+
};
23+
}
24+
25+
it('calls registerDebug on every plugin that implements it', () => {
26+
const logger = createMockLogger();
27+
const debugOverride = createMockDebugOverride();
28+
const mockClient = { id: 'test-client' };
29+
30+
const plugin1: LDPluginBase<typeof mockClient, unknown> = {
31+
getMetadata: jest.fn().mockReturnValue({ name: 'plugin1' }),
32+
register: jest.fn(),
33+
registerDebug: jest.fn(),
34+
};
35+
36+
const plugin2: LDPluginBase<typeof mockClient, unknown> = {
37+
getMetadata: jest.fn().mockReturnValue({ name: 'plugin2' }),
38+
register: jest.fn(),
39+
registerDebug: jest.fn(),
40+
};
41+
42+
safeRegisterDebugOverridePlugins(logger, debugOverride, [plugin1, plugin2]);
43+
44+
expect(plugin1.registerDebug).toHaveBeenCalledWith(debugOverride);
45+
expect(plugin2.registerDebug).toHaveBeenCalledWith(debugOverride);
46+
expect(logger.error).not.toHaveBeenCalled();
47+
});
48+
49+
it('skips plugins that do not implement registerDebug', () => {
50+
const logger = createMockLogger();
51+
const debugOverride = createMockDebugOverride();
52+
const mockClient = { id: 'test-client' };
53+
54+
const pluginWithDebug: LDPluginBase<typeof mockClient, unknown> = {
55+
getMetadata: jest.fn().mockReturnValue({ name: 'with-debug' }),
56+
register: jest.fn(),
57+
registerDebug: jest.fn(),
58+
};
59+
60+
const pluginWithoutDebug: LDPluginBase<typeof mockClient, unknown> = {
61+
getMetadata: jest.fn().mockReturnValue({ name: 'no-debug' }),
62+
register: jest.fn(),
63+
};
64+
65+
safeRegisterDebugOverridePlugins(logger, debugOverride, [
66+
pluginWithoutDebug,
67+
pluginWithDebug,
68+
]);
69+
70+
expect(pluginWithDebug.registerDebug).toHaveBeenCalledWith(debugOverride);
71+
expect(logger.error).not.toHaveBeenCalled();
72+
});
73+
74+
it('continues processing and logs error when registerDebug throws', () => {
75+
const logger = createMockLogger();
76+
const debugOverride = createMockDebugOverride();
77+
const mockClient = { id: 'test-client' };
78+
79+
const throwingPlugin: LDPluginBase<typeof mockClient, unknown> = {
80+
getMetadata: jest.fn().mockReturnValue({ name: 'error-plugin' }),
81+
register: jest.fn(),
82+
registerDebug: jest.fn().mockImplementation(() => {
83+
throw new Error('register-debug failure');
84+
}),
85+
};
86+
87+
const workingPlugin: LDPluginBase<typeof mockClient, unknown> = {
88+
getMetadata: jest.fn().mockReturnValue({ name: 'working-plugin' }),
89+
register: jest.fn(),
90+
registerDebug: jest.fn(),
91+
};
92+
93+
safeRegisterDebugOverridePlugins(logger, debugOverride, [throwingPlugin, workingPlugin]);
94+
95+
expect(throwingPlugin.registerDebug).toHaveBeenCalledWith(debugOverride);
96+
expect(workingPlugin.registerDebug).toHaveBeenCalledWith(debugOverride);
97+
expect(logger.error).toHaveBeenCalledWith(
98+
'Exception thrown registering plugin error-plugin.',
99+
);
100+
});
101+
102+
it('handles an empty plugins array without error', () => {
103+
const logger = createMockLogger();
104+
const debugOverride = createMockDebugOverride();
105+
106+
expect(() =>
107+
safeRegisterDebugOverridePlugins(logger, debugOverride, []),
108+
).not.toThrow();
109+
110+
expect(logger.error).not.toHaveBeenCalled();
111+
});

‎packages/shared/sdk-client/src/LDClientImpl.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import {
5555
import createEventProcessor from './events/createEventProcessor';
5656
import EventFactory from './events/EventFactory';
5757
import { readFlagsFromBootstrap } from './flag-manager/bootstrap';
58-
import DefaultFlagManager, { FlagManager, LDDebugOverride } from './flag-manager/FlagManager';
58+
import DefaultFlagManager, { FlagManager } from './flag-manager/FlagManager';
5959
import { FlagChangeType } from './flag-manager/FlagUpdater';
6060
import { ItemDescriptor } from './flag-manager/ItemDescriptor';
6161
import HookRunner from './HookRunner';
@@ -146,6 +146,12 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
146146
this._config.disableCache ?? false,
147147
this._config.logger,
148148
);
149+
150+
const debugOverride = this._flagManager.getDebugOverride?.();
151+
if (debugOverride && internalOptions?.registerDebugOverrides) {
152+
internalOptions.registerDebugOverrides(debugOverride);
153+
}
154+
149155
this._diagnosticsManager = createDiagnosticsManager(sdkKey, this._config, platform);
150156
this._eventProcessor = createEventProcessor(
151157
sdkKey,
@@ -813,10 +819,6 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
813819
this._eventProcessor?.sendEvent(event);
814820
}
815821

816-
protected getDebugOverrides(): LDDebugOverride | undefined {
817-
return this._flagManager.getDebugOverride?.();
818-
}
819-
820822
private _handleInspectionChanged(flagKeys: Array<string>, type: FlagChangeType) {
821823
if (!this._inspectorManager.hasInspectors()) {
822824
return;

‎packages/shared/sdk-client/src/api/LDDebugOverride.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { LDFlagValue } from '@launchdarkly/js-sdk-common';
2+
3+
import { ItemDescriptor } from '../flag-manager/ItemDescriptor';
4+
5+
/**
6+
* Debug interface for plugins that need to override flag values during development.
7+
* This interface provides methods to temporarily override flag values that take
8+
* precedence over the actual flag values from LaunchDarkly. These overrides are
9+
* useful for testing, development, and debugging scenarios.
10+
*
11+
* @experimental This interface is experimental and intended for use by LaunchDarkly tools at this time.
12+
* The API may change in future versions.
13+
*/
14+
export interface LDDebugOverride {
15+
/**
16+
* Set an override value for a flag that takes precedence over the real flag value.
17+
*
18+
* @param flagKey The flag key.
19+
* @param value The override value.
20+
*/
21+
setOverride(flagKey: string, value: LDFlagValue): void;
22+
23+
/**
24+
* Remove an override value for a flag, reverting to the real flag value.
25+
*
26+
* @param flagKey The flag key.
27+
*/
28+
removeOverride(flagKey: string): void;
29+
30+
/**
31+
* Clear all override values, reverting all flags to their real values.
32+
*/
33+
clearAllOverrides(): void;
34+
35+
/**
36+
* Get all currently active flag overrides.
37+
*
38+
* @returns
39+
* An object containing all active overrides as key-value pairs,
40+
* where keys are flag keys and values are the overridden flag values.
41+
* Returns an empty object if no overrides are active.
42+
*/
43+
getAllOverrides(): { [key: string]: ItemDescriptor };
44+
}

‎packages/shared/sdk-client/src/api/LDPlugin.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { LDPluginBase as LDPluginBaseCommon } from '@launchdarkly/js-sdk-common';
22

3-
import { LDDebugOverride } from '../flag-manager/FlagManager';
3+
import { LDDebugOverride } from './LDDebugOverride';
44

55
export interface LDPluginBase<TClient, THook> extends LDPluginBaseCommon<TClient, THook> {
66
/**

‎packages/shared/sdk-client/src/configuration/Configuration.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import { Hook, type LDOptions } from '../api';
1313
import { LDContext } from '../api/LDContext';
1414
import { LDInspection } from '../api/LDInspection';
15+
import type { LDDebugOverride } from '../api/LDDebugOverride';
1516
import type {
1617
InternalDataSystemOptions,
1718
PlatformDataSystemDefaults,
@@ -27,6 +28,7 @@ export interface LDClientInternalOptions extends internal.LDInternalOptions {
2728
credentialType: 'clientSideId' | 'mobileKey';
2829
getLegacyStorageKeys?: () => string[];
2930
dataSystemDefaults?: PlatformDataSystemDefaults;
31+
registerDebugOverrides?: (debugOverride: LDDebugOverride) => void;
3032

3133
/**
3234
* When true, the SDK requires `start()` to be called before `identify()`.

‎packages/shared/sdk-client/src/flag-manager/FlagManager.ts‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Context, internal, LDFlagValue, LDLogger, Platform } from '@launchdarkly/js-sdk-common';
22

3+
import { LDDebugOverride } from '../api/LDDebugOverride';
34
import { namespaceForEnvironment } from '../storage/namespaceUtils';
45
import FlagPersistence from './FlagPersistence';
56
import { createDefaultFlagStore } from './FlagStore';
@@ -90,47 +91,6 @@ export interface FlagManager {
9091
getDebugOverride?(): LDDebugOverride;
9192
}
9293

93-
/**
94-
* Debug interface for plugins that need to override flag values during development.
95-
* This interface provides methods to temporarily override flag values that take
96-
* precedence over the actual flag values from LaunchDarkly. These overrides are
97-
* useful for testing, development, and debugging scenarios.
98-
*
99-
* @experimental This interface is experimental and intended for use by LaunchDarkly tools at this time.
100-
* The API may change in future versions.
101-
*/
102-
export interface LDDebugOverride {
103-
/**
104-
* Set an override value for a flag that takes precedence over the real flag value.
105-
*
106-
* @param flagKey The flag key.
107-
* @param value The override value.
108-
*/
109-
setOverride(flagKey: string, value: LDFlagValue): void;
110-
111-
/**
112-
* Remove an override value for a flag, reverting to the real flag value.
113-
*
114-
* @param flagKey The flag key.
115-
*/
116-
removeOverride(flagKey: string): void;
117-
118-
/**
119-
* Clear all override values, reverting all flags to their real values.
120-
*/
121-
clearAllOverrides(): void;
122-
123-
/**
124-
* Get all currently active flag overrides.
125-
*
126-
* @returns
127-
* An object containing all active overrides as key-value pairs,
128-
* where keys are flag keys and values are the overridden flag values.
129-
* Returns an empty object if no overrides are active.
130-
*/
131-
getAllOverrides(): { [key: string]: ItemDescriptor };
132-
}
133-
13494
export default class DefaultFlagManager implements FlagManager {
13595
private _flagStore = createDefaultFlagStore();
13696
private _flagUpdater: FlagUpdater;

‎packages/shared/sdk-client/src/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ export type {
4848
} from './api';
4949

5050
export type { DataManager, DataManagerFactory, ConnectionParams } from './DataManager';
51-
export type { FlagManager, LDDebugOverride } from './flag-manager/FlagManager';
51+
export type { FlagManager } from './flag-manager/FlagManager';
52+
export type { LDDebugOverride } from './api/LDDebugOverride';
5253
export { safeRegisterDebugOverridePlugins } from './plugins/safeRegisterDebugOverridePlugins';
5354
export type { Configuration } from './configuration/Configuration';
5455
export { default as validateOptions } from './configuration/validateOptions';

‎packages/shared/sdk-client/src/plugins/safeRegisterDebugOverridePlugins.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { internal, LDLogger } from '@launchdarkly/js-sdk-common';
22

33
import { LDPluginBase } from '../api';
4-
import { LDDebugOverride } from '../flag-manager/FlagManager';
4+
import { LDDebugOverride } from '../api/LDDebugOverride';
55

66
/**
77
* Safe register debug override plugins.

0 commit comments

Comments
 (0)