Skip to content

[FSSDK-11035] add logger factory and tests #985

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.

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
merged 5 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
35 changes: 35 additions & 0 deletions lib/error/error_notifier.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { describe, it, expect, vi } from 'vitest';

import { DefaultErrorNotifier } from './error_notifier';
import { OptimizelyError } from './optimizly_error';

const mockMessageResolver = (prefix = '') => {
return {
resolve: vi.fn().mockImplementation((message) => `${prefix} ${message}`),
};
}

describe('DefaultErrorNotifier', () => {
it('should call the error handler with the error if the error is not an OptimizelyError', () => {
const errorHandler = { handleError: vi.fn() };
const messageResolver = mockMessageResolver();
const errorNotifier = new DefaultErrorNotifier(errorHandler, messageResolver);

const error = new Error('error');
errorNotifier.notify(error);

expect(errorHandler.handleError).toHaveBeenCalledWith(error);
});

it.only('should resolve the message of an OptimizelyError before calling the error handler', () => {
const errorHandler = { handleError: vi.fn() };
const messageResolver = mockMessageResolver('err');
const errorNotifier = new DefaultErrorNotifier(errorHandler, messageResolver);

const error = new OptimizelyError('test %s', 'one');
errorNotifier.notify(error);

expect(errorHandler.handleError).toHaveBeenCalledWith(error);
expect(error.message).toBe('err test one');
});
});
19 changes: 19 additions & 0 deletions lib/error/error_notifier_factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { errorResolver } from "../message/message_resolver";
import { ErrorHandler } from "./error_handler";
import { DefaultErrorNotifier } from "./error_notifier";

const errorNotifierSymbol = Symbol();

export type OpaqueErrorNotifier = {
[errorNotifierSymbol]: unknown;
};

export const createErrorNotifier = (errorHandler: ErrorHandler): OpaqueErrorNotifier => {
return {
[errorNotifierSymbol]: new DefaultErrorNotifier(errorHandler, errorResolver),
}
}

export const extractErrorNotifier = (errorNotifier: OpaqueErrorNotifier): DefaultErrorNotifier => {
return errorNotifier[errorNotifierSymbol] as DefaultErrorNotifier;
}
45 changes: 45 additions & 0 deletions lib/error/error_reporter.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { describe, it, expect, vi } from 'vitest';

import { ErrorReporter } from './error_reporter';

import { OptimizelyError } from './optimizly_error';

const mockMessageResolver = (prefix = '') => {
return {
resolve: vi.fn().mockImplementation((message) => `${prefix} ${message}`),
};
}

describe('ErrorReporter', () => {
it('should call the logger and errorNotifier with the first argument if it is an Error object', () => {
const logger = { error: vi.fn() };
const errorNotifier = { notify: vi.fn() };
const errorReporter = new ErrorReporter(logger as any, errorNotifier as any);

const error = new Error('error');
errorReporter.report(error);

expect(logger.error).toHaveBeenCalledWith(error);
expect(errorNotifier.notify).toHaveBeenCalledWith(error);
});

it('should create an OptimizelyError and call the logger and errorNotifier with it if the first argument is a string', () => {
const logger = { error: vi.fn() };
const errorNotifier = { notify: vi.fn() };
const errorReporter = new ErrorReporter(logger as any, errorNotifier as any);

errorReporter.report('message', 1, 2);

expect(logger.error).toHaveBeenCalled();
const loggedError = logger.error.mock.calls[0][0];
expect(loggedError).toBeInstanceOf(OptimizelyError);
expect(loggedError.baseMessage).toBe('message');
expect(loggedError.params).toEqual([1, 2]);

expect(errorNotifier.notify).toHaveBeenCalled();
const notifiedError = errorNotifier.notify.mock.calls[0][0];
expect(notifiedError).toBeInstanceOf(OptimizelyError);
expect(notifiedError.baseMessage).toBe('message');
expect(notifiedError.params).toEqual([1, 2]);
});
});
4 changes: 2 additions & 2 deletions lib/error/optimizly_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { MessageResolver } from "../message/message_resolver";
import { sprintf } from "../utils/fns";

export class OptimizelyError extends Error {
private baseMessage: string;
private params: any[];
baseMessage: string;
params: any[];
private resolved = false;
constructor(baseMessage: string, ...params: any[]) {
super();
Expand Down
16 changes: 14 additions & 2 deletions lib/index.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ import { createBatchEventProcessor, createForwardingEventProcessor } from './eve
import { createVuidManager } from './vuid/vuid_manager_factory.browser';
import { createOdpManager } from './odp/odp_manager_factory.browser';
import { ODP_DISABLED, UNABLE_TO_ATTACH_UNLOAD } from './log_messages';
import { extractLogger, createLogger } from './logging/logger_factory';
import { extractErrorNotifier, createErrorNotifier } from './error/error_notifier_factory';
import { LoggerFacade } from './logging/logger';
import { Maybe } from './utils/type';


const MODULE_NAME = 'INDEX_BROWSER';
Expand All @@ -47,15 +51,21 @@ let hasRetriedEvents = false;
* null on error
*/
const createInstance = function(config: Config): Client | null {
let logger: Maybe<LoggerFacade>;

try {
configValidator.validate(config);

const { clientEngine, clientVersion } = config;
logger = config.logger ? extractLogger(config.logger) : undefined;
const errorNotifier = config.errorNotifier ? extractErrorNotifier(config.errorNotifier) : undefined;

const optimizelyOptions: OptimizelyOptions = {
...config,
clientEngine: clientEngine || enums.JAVASCRIPT_CLIENT_ENGINE,
clientVersion: clientVersion || enums.CLIENT_VERSION,
logger,
errorNotifier,
};

const optimizely = new Optimizely(optimizelyOptions);
Expand All @@ -73,13 +83,13 @@ const createInstance = function(config: Config): Client | null {
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (e) {
config.logger?.error(UNABLE_TO_ATTACH_UNLOAD, e.message);
logger?.error(UNABLE_TO_ATTACH_UNLOAD, e.message);
}

return optimizely;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (e) {
config.logger?.error(e);
logger?.error(e);
return null;
}
};
Expand All @@ -103,6 +113,8 @@ export {
createBatchEventProcessor,
createOdpManager,
createVuidManager,
createLogger,
createErrorNotifier,
};

export * from './common_exports';
Expand Down
65 changes: 1 addition & 64 deletions lib/index.lite.ts
Original file line number Diff line number Diff line change
@@ -1,64 +1 @@
/**
* Copyright 2021-2022, 2024, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import configValidator from './utils/config_validator';
import defaultErrorHandler from './plugins/error_handler';
import * as enums from './utils/enums';
import Optimizely from './optimizely';
import { createNotificationCenter } from './notification_center';
import { OptimizelyDecideOption, Client, Config } from './shared_types';
import * as commonExports from './common_exports';

/**
* Creates an instance of the Optimizely class
* @param {ConfigLite} config
* @return {Client|null} the Optimizely client object
* null on error
*/
const createInstance = function(config: Config): Client | null {
try {
configValidator.validate(config);

const optimizelyOptions = {
clientEngine: enums.JAVASCRIPT_CLIENT_ENGINE,
...config,
};

const optimizely = new Optimizely(optimizelyOptions);
return optimizely;
} catch (e: any) {
config.logger?.error(e);
return null;
}
};

export {
defaultErrorHandler as errorHandler,
enums,
createInstance,
OptimizelyDecideOption,
};

export * from './common_exports';

export default {
...commonExports,
errorHandler: defaultErrorHandler,
enums,
createInstance,
OptimizelyDecideOption,
};

export * from './export_types'
const msg = 'not used';
4 changes: 2 additions & 2 deletions lib/index.node.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ describe('optimizelyFactory', function() {
// sinon.assert.calledWith(localLogger.log, enums.LOG_LEVEL.ERROR);
// });

it('should not throw if the provided config is not valid and log an error if no logger is provided', function() {
it('should not throw if the provided config is not valid', function() {
configValidator.validate.throws(new Error(INVALID_CONFIG_OR_SOMETHING));
assert.doesNotThrow(function() {
var optlyInstance = optimizelyFactory.createInstance({
projectConfigManager: getMockProjectConfigManager(),
logger: fakeLogger,
});
});
sinon.assert.calledOnce(fakeLogger.error);
// sinon.assert.calledOnce(fakeLogger.error);
});

// it('should create an instance of optimizely', function() {
Expand Down
15 changes: 14 additions & 1 deletion lib/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import { createVuidManager } from './vuid/vuid_manager_factory.node';
import { createOdpManager } from './odp/odp_manager_factory.node';
import { ODP_DISABLED } from './log_messages';
import { create } from 'domain';
import { extractLogger, createLogger } from './logging/logger_factory';
import { extractErrorNotifier, createErrorNotifier } from './error/error_notifier_factory';
import { Maybe } from './utils/type';
import { LoggerFacade } from './logging/logger';
import { ErrorNotifier } from './error/error_notifier';

const DEFAULT_EVENT_BATCH_SIZE = 10;
const DEFAULT_EVENT_FLUSH_INTERVAL = 30000; // Unit is ms, default is 30s
Expand All @@ -41,21 +46,27 @@ const DEFAULT_EVENT_MAX_QUEUE_SIZE = 10000;
* null on error
*/
const createInstance = function(config: Config): Client | null {
let logger: Maybe<LoggerFacade>;

try {
configValidator.validate(config);

const { clientEngine, clientVersion } = config;
logger = config.logger ? extractLogger(config.logger) : undefined;
const errorNotifier = config.errorNotifier ? extractErrorNotifier(config.errorNotifier) : undefined;

const optimizelyOptions = {
...config,
clientEngine: clientEngine || enums.NODE_CLIENT_ENGINE,
clientVersion: clientVersion || enums.CLIENT_VERSION,
logger,
errorNotifier,
};

return new Optimizely(optimizelyOptions);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (e) {
config.logger?.error(e);
logger?.error(e);
return null;
}
};
Expand All @@ -74,6 +85,8 @@ export {
createBatchEventProcessor,
createOdpManager,
createVuidManager,
createLogger,
createErrorNotifier,
};

export * from './common_exports';
Expand Down
8 changes: 4 additions & 4 deletions lib/index.react_native.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ describe('javascript-sdk/react-native', () => {
it('should create an instance of optimizely', () => {
const optlyInstance = optimizelyFactory.createInstance({
projectConfigManager: getMockProjectConfigManager(),
errorHandler: fakeErrorHandler,
// errorHandler: fakeErrorHandler,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
logger: mockLogger,
// logger: mockLogger,
});

expect(optlyInstance).toBeInstanceOf(Optimizely);
Expand All @@ -97,10 +97,10 @@ describe('javascript-sdk/react-native', () => {
it('should set the React Native JS client engine and javascript SDK version', () => {
const optlyInstance = optimizelyFactory.createInstance({
projectConfigManager: getMockProjectConfigManager(),
errorHandler: fakeErrorHandler,
// errorHandler: fakeErrorHandler,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
logger: mockLogger,
// logger: mockLogger,
});
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
Expand Down
15 changes: 14 additions & 1 deletion lib/index.react_native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import { createVuidManager } from './vuid/vuid_manager_factory.react_native';

import 'fast-text-encoding';
import 'react-native-get-random-values';
import { Maybe } from './utils/type';
import { LoggerFacade } from './logging/logger';
import { extractLogger, createLogger } from './logging/logger_factory';
import { extractErrorNotifier, createErrorNotifier } from './error/error_notifier_factory';

const DEFAULT_EVENT_BATCH_SIZE = 10;
const DEFAULT_EVENT_FLUSH_INTERVAL = 1000; // Unit is ms, default is 1s
Expand All @@ -41,15 +45,22 @@ const DEFAULT_EVENT_MAX_QUEUE_SIZE = 10000;
* null on error
*/
const createInstance = function(config: Config): Client | null {
let logger: Maybe<LoggerFacade>;

try {
configValidator.validate(config);

const { clientEngine, clientVersion } = config;

logger = config.logger ? extractLogger(config.logger) : undefined;
const errorNotifier = config.errorNotifier ? extractErrorNotifier(config.errorNotifier) : undefined;

const optimizelyOptions = {
...config,
clientEngine: clientEngine || enums.REACT_NATIVE_JS_CLIENT_ENGINE,
clientVersion: clientVersion || enums.CLIENT_VERSION,
logger,
errorNotifier,
};

// If client engine is react, convert it to react native.
Expand All @@ -60,7 +71,7 @@ const createInstance = function(config: Config): Client | null {
return new Optimizely(optimizelyOptions);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (e) {
config.logger?.error(e);
logger?.error(e);
return null;
}
};
Expand All @@ -79,6 +90,8 @@ export {
createBatchEventProcessor,
createOdpManager,
createVuidManager,
createLogger,
createErrorNotifier,
};

export * from './common_exports';
Expand Down
Loading
Loading