Skip to content

Commit b53e512

Browse files
author
Athira M
committed
Add error handling
1 parent bbc8f4d commit b53e512

File tree

4 files changed

+37
-18
lines changed

4 files changed

+37
-18
lines changed

packages/remote-config/src/abt/experiment.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,19 @@ import { FirebaseExperimentDescription } from '../public_types';
1919
import { Provider } from '@firebase/component';
2020
import { FirebaseAnalyticsInternalName } from '@firebase/analytics-interop-types';
2121
import { Logger } from '@firebase/logger';
22+
import { RemoteConfig } from '../remote_config';
23+
import { ERROR_FACTORY, ErrorCode } from '../errors';
2224

2325
export class Experiment {
24-
constructor(
25-
private readonly storage: Storage,
26-
private readonly logger: Logger,
27-
private readonly analyticsProvider: Provider<FirebaseAnalyticsInternalName>
28-
) {}
26+
private storage: Storage;
27+
private logger: Logger;
28+
private analyticsProvider: Provider<FirebaseAnalyticsInternalName>;
29+
30+
constructor(rc: RemoteConfig) {
31+
this.storage = rc._storage;
32+
this.logger = rc._logger;
33+
this.analyticsProvider = rc._analyticsProvider;
34+
}
2935

3036
async updateActiveExperiments(
3137
latestExperiments: FirebaseExperimentDescription[]
@@ -58,7 +64,7 @@ export class Experiment {
5864
customProperty[experimentId] = experimentInfo.variantId;
5965
}
6066
}
61-
void this.addExperimentToAnalytics(customProperty);
67+
this.addExperimentToAnalytics(customProperty);
6268
}
6369

6470
private removeInactiveExperiments(
@@ -71,22 +77,29 @@ export class Experiment {
7177
customProperty[experimentId] = null;
7278
}
7379
}
74-
void this.addExperimentToAnalytics(customProperty);
80+
this.addExperimentToAnalytics(customProperty);
7581
}
7682

77-
private async addExperimentToAnalytics(
83+
private addExperimentToAnalytics(
7884
customProperty: Record<string, string | null>
79-
): Promise<void> {
85+
): void {
86+
if (Object.keys(customProperty).length === 0) {
87+
return;
88+
}
8089
try {
8190
const analytics = this.analyticsProvider.getImmediate({ optional: true });
8291
if (analytics) {
8392
analytics.setUserProperties({ properties: customProperty });
8493
} else {
94+
// TODO: Update warning message
8595
this.logger.warn(`Analytics is not imported correctly`);
8696
}
8797
} catch (error) {
98+
// TODO: Update error message
8899
this.logger.error(`Failed to add experiment to analytics : ${error}`);
89-
return;
100+
throw ERROR_FACTORY.create(ErrorCode.ANALYTICS_UNAVAILABLE, {
101+
originalErrorMessage: (error as Error)?.message
102+
});
90103
}
91104
}
92105
}

packages/remote-config/src/api.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,7 @@ export async function activate(remoteConfig: RemoteConfig): Promise<boolean> {
111111
// config.
112112
return false;
113113
}
114-
const experiment = new Experiment(
115-
rc._storage,
116-
rc._logger,
117-
rc._analyticsProvider
118-
);
114+
const experiment = new Experiment(rc);
119115
const updateActiveExperiments = lastSuccessfulFetchResponse.experiments
120116
? experiment.updateActiveExperiments(
121117
lastSuccessfulFetchResponse.experiments

packages/remote-config/src/errors.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ export const enum ErrorCode {
3737
CONFIG_UPDATE_STREAM_ERROR = 'stream-error',
3838
CONFIG_UPDATE_UNAVAILABLE = 'realtime-unavailable',
3939
CONFIG_UPDATE_MESSAGE_INVALID = 'update-message-invalid',
40-
CONFIG_UPDATE_NOT_FETCHED = 'update-not-fetched'
40+
CONFIG_UPDATE_NOT_FETCHED = 'update-not-fetched',
41+
ANALYTICS_UNAVAILABLE = 'analytics-unavailable'
4142
}
4243

4344
const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
@@ -84,7 +85,9 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
8485
[ErrorCode.CONFIG_UPDATE_MESSAGE_INVALID]:
8586
'The stream invalidation message was unparsable: {$originalErrorMessage}',
8687
[ErrorCode.CONFIG_UPDATE_NOT_FETCHED]:
87-
'Unable to fetch the latest config: {$originalErrorMessage}'
88+
'Unable to fetch the latest config: {$originalErrorMessage}',
89+
[ErrorCode.ANALYTICS_UNAVAILABLE]:
90+
'Connection to firebase analytics failed: {$originalErrorMessage}'
8891
};
8992

9093
// Note this is effectively a type system binding a code to params. This approach overlaps with the
@@ -108,6 +111,7 @@ interface ErrorParams {
108111
[ErrorCode.CONFIG_UPDATE_UNAVAILABLE]: { originalErrorMessage: string };
109112
[ErrorCode.CONFIG_UPDATE_MESSAGE_INVALID]: { originalErrorMessage: string };
110113
[ErrorCode.CONFIG_UPDATE_NOT_FETCHED]: { originalErrorMessage: string };
114+
[ErrorCode.ANALYTICS_UNAVAILABLE]: { originalErrorMessage: string };
111115
}
112116

113117
export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(

packages/remote-config/test/abt/experiment.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,18 @@ import { Storage } from '../../src/storage/storage';
2323
import { Provider } from '@firebase/component';
2424
import { FirebaseAnalyticsInternalName } from '@firebase/analytics-interop-types';
2525
import { Logger } from '@firebase/logger';
26+
import { RemoteConfig } from '../../src/remote_config';
2627

2728
describe('Experiment', () => {
2829
const storage = {} as Storage;
2930
const analyticsProvider = {} as Provider<FirebaseAnalyticsInternalName>;
3031
const logger = {} as Logger;
31-
const experiment = new Experiment(storage, logger, analyticsProvider);
32+
const rc = {
33+
_storage: storage,
34+
_analyticsProvider: analyticsProvider,
35+
_logger: logger
36+
} as RemoteConfig;
37+
const experiment = new Experiment(rc);
3238

3339
describe('updateActiveExperiments', () => {
3440
beforeEach(() => {

0 commit comments

Comments
 (0)