Skip to content

Commit 3ee2141

Browse files
davidchindavidchin
committed
fix(core): CHECKOUT-9450 Pass error logger instead of using global Sentry instance
1 parent 77266d1 commit 3ee2141

13 files changed

+88
-24
lines changed

‎packages/core/src/checkout/checkout-service.spec.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
} from '../billing';
1919
import { getBillingAddress } from '../billing/billing-addresses.mock';
2020
import { createDataStoreProjection, DataStoreProjection } from '../common/data-store';
21-
import { ErrorActionCreator } from '../common/error';
21+
import { ErrorActionCreator, ErrorLogger } from '../common/error';
2222
import { getResponse } from '../common/http-request/responses.mock';
2323
import { ResolveIdRegistry } from '../common/registry';
2424
import { ConfigActionCreator, ConfigRequestSender } from '../config';
@@ -376,10 +376,13 @@ describe('CheckoutService', () => {
376376
checkoutRequestSender,
377377
);
378378

379+
const errorLogger: ErrorLogger = { log: jest.fn() };
380+
379381
customerStrategyActionCreator = new CustomerStrategyActionCreator(
380382
createCustomerStrategyRegistry(store, requestSender, locale),
381383
customerRegistryV2,
382384
paymentIntegrationService,
385+
errorLogger,
383386
);
384387

385388
instrumentActionCreator = new InstrumentActionCreator(instrumentRequestSender);
@@ -399,6 +402,7 @@ describe('CheckoutService', () => {
399402
orderActionCreator,
400403
spamProtectionActionCreator,
401404
paymentIntegrationService,
405+
errorLogger,
402406
);
403407

404408
shippingStrategyActionCreator = new ShippingStrategyActionCreator(

‎packages/core/src/checkout/create-checkout-service.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createScriptLoader } from '@bigcommerce/script-loader';
33

44
import { BillingAddressActionCreator, BillingAddressRequestSender } from '../billing';
55
import { createDataStoreProjection } from '../common/data-store';
6-
import { ErrorActionCreator } from '../common/error';
6+
import { DefaultErrorLogger, ErrorActionCreator, ErrorLogger } from '../common/error';
77
import { getDefaultLogger } from '../common/log';
88
import { getEnvironment } from '../common/utility';
99
import { ConfigActionCreator, ConfigRequestSender, ConfigState, ConfigWindow } from '../config';
@@ -108,7 +108,11 @@ export default function createCheckoutService(options?: CheckoutServiceOptions):
108108
errors: {},
109109
statuses: {},
110110
};
111-
const { locale = '', shouldWarnMutation = true } = options || {};
111+
const {
112+
locale = '',
113+
shouldWarnMutation = true,
114+
errorLogger = new DefaultErrorLogger(),
115+
} = options || {};
112116
const requestSender = createRequestSender({ host: options && options.host });
113117
const store = createCheckoutStore({ config }, { shouldWarnMutation });
114118
const paymentClient = createPaymentClient(store);
@@ -184,6 +188,7 @@ export default function createCheckoutService(options?: CheckoutServiceOptions):
184188
createCustomerStrategyRegistry(store, requestSender, locale),
185189
customerRegistryV2,
186190
paymentIntegrationService,
191+
errorLogger,
187192
),
188193
new ErrorActionCreator(),
189194
new GiftCertificateActionCreator(new GiftCertificateRequestSender(requestSender)),
@@ -197,11 +202,13 @@ export default function createCheckoutService(options?: CheckoutServiceOptions):
197202
requestSender,
198203
spamProtection,
199204
locale,
205+
errorLogger,
200206
),
201207
registryV2,
202208
orderActionCreator,
203209
spamProtectionActionCreator,
204210
paymentIntegrationService,
211+
errorLogger,
205212
),
206213
new PickupOptionActionCreator(new PickupOptionRequestSender(requestSender)),
207214
new ShippingCountryActionCreator(
@@ -224,4 +231,5 @@ export interface CheckoutServiceOptions {
224231
host?: string;
225232
shouldWarnMutation?: boolean;
226233
externalSource?: string;
234+
errorLogger?: ErrorLogger;
227235
}

‎packages/core/src/common/error/error-logger.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
export interface ErrorLogger {
2+
log(error: Error): void;
3+
}
4+
5+
export class DefaultErrorLogger implements ErrorLogger {
6+
log(error: Error): void {
7+
// eslint-disable-next-line no-console
8+
console.error(error);
9+
}
10+
}

‎packages/core/src/common/error/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export * from './error-actions';
2+
export * from './error-logger';
23

34
export { default as clearErrorReducer } from './clear-error-reducer';
45
export { default as createRequestErrorFactory } from './create-request-error-factory';

‎packages/core/src/common/types/sentry.ts‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎packages/core/src/customer/customer-strategy-action-creator.spec.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '../checkout';
1616
import { getCheckoutStoreState } from '../checkout/checkouts.mock';
1717
import { MutationObserverFactory } from '../common/dom';
18+
import { ErrorLogger } from '../common/error';
1819
import { Registry } from '../common/registry';
1920
import { ConfigActionCreator, ConfigRequestSender } from '../config';
2021
import { FormFieldsActionCreator, FormFieldsRequestSender } from '../form';
@@ -44,6 +45,7 @@ describe('CustomerStrategyActionCreator', () => {
4445
let store: CheckoutStore;
4546
let strategy: DefaultCustomerStrategy;
4647
let paymentIntegrationService: PaymentIntegrationService;
48+
let errorLogger: ErrorLogger;
4749

4850
beforeEach(() => {
4951
const requestSender = createRequestSender();
@@ -69,6 +71,9 @@ describe('CustomerStrategyActionCreator', () => {
6971
store = createCheckoutStore(state);
7072

7173
paymentIntegrationService = createPaymentIntegrationService(store);
74+
errorLogger = {
75+
log: jest.fn(),
76+
};
7277

7378
customerRegistryV2 = createCustomerStrategyRegistryV2(paymentIntegrationService, {});
7479
registry = createCustomerStrategyRegistry(store, createRequestSender(), 'en');
@@ -105,6 +110,7 @@ describe('CustomerStrategyActionCreator', () => {
105110
registry,
106111
customerRegistryV2,
107112
paymentIntegrationService,
113+
errorLogger,
108114
);
109115

110116
await from(actionCreator.initialize({ methodId: 'default' })(store))
@@ -119,6 +125,7 @@ describe('CustomerStrategyActionCreator', () => {
119125
registry,
120126
customerRegistryV2,
121127
paymentIntegrationService,
128+
errorLogger,
122129
);
123130
const options = { methodId: 'default' };
124131

@@ -132,6 +139,7 @@ describe('CustomerStrategyActionCreator', () => {
132139
registry,
133140
customerRegistryV2,
134141
paymentIntegrationService,
142+
errorLogger,
135143
);
136144
const strategy = registry.get('amazon');
137145

@@ -147,6 +155,7 @@ describe('CustomerStrategyActionCreator', () => {
147155
registry,
148156
customerRegistryV2,
149157
paymentIntegrationService,
158+
errorLogger,
150159
);
151160
const actions = await from(actionCreator.initialize({ methodId: 'default' })(store))
152161
.pipe(toArray())
@@ -169,6 +178,7 @@ describe('CustomerStrategyActionCreator', () => {
169178
registry,
170179
customerRegistryV2,
171180
paymentIntegrationService,
181+
errorLogger,
172182
);
173183
const initializeError = new Error();
174184
const errorHandler = jest.fn((action) => of(action));
@@ -211,6 +221,7 @@ describe('CustomerStrategyActionCreator', () => {
211221
registry,
212222
customerRegistryV2,
213223
paymentIntegrationService,
224+
errorLogger,
214225
);
215226

216227
await from(actionCreator.deinitialize({ methodId: 'default' })(store))
@@ -225,6 +236,7 @@ describe('CustomerStrategyActionCreator', () => {
225236
registry,
226237
customerRegistryV2,
227238
paymentIntegrationService,
239+
errorLogger,
228240
);
229241
const options = { methodId: 'default' };
230242

@@ -238,6 +250,7 @@ describe('CustomerStrategyActionCreator', () => {
238250
registry,
239251
customerRegistryV2,
240252
paymentIntegrationService,
253+
errorLogger,
241254
);
242255
const strategy = registry.get('amazon');
243256

@@ -253,6 +266,7 @@ describe('CustomerStrategyActionCreator', () => {
253266
registry,
254267
customerRegistryV2,
255268
paymentIntegrationService,
269+
errorLogger,
256270
);
257271
const actions = await from(actionCreator.deinitialize({ methodId: 'default' })(store))
258272
.pipe(toArray())
@@ -275,6 +289,7 @@ describe('CustomerStrategyActionCreator', () => {
275289
registry,
276290
customerRegistryV2,
277291
paymentIntegrationService,
292+
errorLogger,
278293
);
279294
const deinitializeError = new Error();
280295
const errorHandler = jest.fn((action) => of(action));
@@ -311,6 +326,7 @@ describe('CustomerStrategyActionCreator', () => {
311326
registry,
312327
customerRegistryV2,
313328
paymentIntegrationService,
329+
errorLogger,
314330
);
315331

316332
await actionCreator
@@ -326,6 +342,7 @@ describe('CustomerStrategyActionCreator', () => {
326342
registry,
327343
customerRegistryV2,
328344
paymentIntegrationService,
345+
errorLogger,
329346
);
330347
const credentials = { email: '[email protected]', password: 'password1' };
331348
const options = { methodId: 'default' };
@@ -340,6 +357,7 @@ describe('CustomerStrategyActionCreator', () => {
340357
registry,
341358
customerRegistryV2,
342359
paymentIntegrationService,
360+
errorLogger,
343361
);
344362
const actions = await actionCreator
345363
.signIn({ email: '[email protected]', password: 'password1' }, { methodId: 'default' })
@@ -357,6 +375,7 @@ describe('CustomerStrategyActionCreator', () => {
357375
registry,
358376
customerRegistryV2,
359377
paymentIntegrationService,
378+
errorLogger,
360379
);
361380
const signInError = new Error();
362381
const errorHandler = jest.fn((action) => of(action));
@@ -391,6 +410,7 @@ describe('CustomerStrategyActionCreator', () => {
391410
registry,
392411
customerRegistryV2,
393412
paymentIntegrationService,
413+
errorLogger,
394414
);
395415

396416
await actionCreator.signOut({ methodId: 'default' }).pipe(toArray()).toPromise();
@@ -403,6 +423,7 @@ describe('CustomerStrategyActionCreator', () => {
403423
registry,
404424
customerRegistryV2,
405425
paymentIntegrationService,
426+
errorLogger,
406427
);
407428
const options = { methodId: 'default' };
408429

@@ -416,6 +437,7 @@ describe('CustomerStrategyActionCreator', () => {
416437
registry,
417438
customerRegistryV2,
418439
paymentIntegrationService,
440+
errorLogger,
419441
);
420442
const actions = await actionCreator
421443
.signOut({ methodId: 'default' })
@@ -439,6 +461,7 @@ describe('CustomerStrategyActionCreator', () => {
439461
registry,
440462
customerRegistryV2,
441463
paymentIntegrationService,
464+
errorLogger,
442465
);
443466
const signOutError = new Error();
444467
const errorHandler = jest.fn((action) => of(action));
@@ -472,6 +495,7 @@ describe('CustomerStrategyActionCreator', () => {
472495
registry,
473496
customerRegistryV2,
474497
paymentIntegrationService,
498+
errorLogger,
475499
);
476500
const options = { methodId: 'default' };
477501
const fakeMethod = jest.fn(() => Promise.resolve());
@@ -486,6 +510,7 @@ describe('CustomerStrategyActionCreator', () => {
486510
registry,
487511
customerRegistryV2,
488512
paymentIntegrationService,
513+
errorLogger,
489514
);
490515
const actions = await actionCreator
491516
.widgetInteraction(
@@ -512,6 +537,7 @@ describe('CustomerStrategyActionCreator', () => {
512537
registry,
513538
customerRegistryV2,
514539
paymentIntegrationService,
540+
errorLogger,
515541
);
516542
const signInError = new Error();
517543
const errorHandler = jest.fn((action) => of(action));

‎packages/core/src/customer/customer-strategy-action-creator.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
import { isExperimentEnabled } from '@bigcommerce/checkout-sdk/utility';
99

1010
import { InternalCheckoutSelectors } from '../checkout';
11+
import { ErrorLogger } from '../common/error';
1112
import { Registry } from '../common/registry';
1213
import { matchExistingIntegrations, registerIntegrations } from '../payment-integration';
1314

@@ -34,6 +35,7 @@ export default class CustomerStrategyActionCreator {
3435
private _strategyRegistry: Registry<CustomerStrategy>,
3536
private _strategyRegistryV2: CustomerStrategyRegistryV2,
3637
private _paymentIntegrationService: PaymentIntegrationService,
38+
private _errorLogger: ErrorLogger,
3739
) {}
3840

3941
signIn(
@@ -160,6 +162,7 @@ export default class CustomerStrategyActionCreator {
160162
this._strategyRegistryV2,
161163
options?.integrations ?? [],
162164
resolveId,
165+
this._errorLogger,
163166
);
164167
registerIntegrations(
165168
this._strategyRegistryV2,

‎packages/core/src/payment-integration/register-integrations.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import {
66
toResolvableModule,
77
} from '@bigcommerce/checkout-sdk/payment-integration-api';
88

9+
import { ErrorLogger } from '../common/error';
910
import { ResolveIdRegistry } from '../common/registry';
10-
import { SentryWindow } from '../common/types/sentry';
1111

1212
export type StrategyFactory<TStrategy> = (
1313
paymentIntegrationService: PaymentIntegrationService,
@@ -44,6 +44,7 @@ export function matchExistingIntegrations<TStrategy, TResolveId extends { [key:
4444
registry: ResolveIdRegistry<TStrategy, TResolveId>,
4545
integrations: Array<StrategyFactory<TStrategy>>,
4646
resolveId: TResolveId,
47+
errorLogger: ErrorLogger,
4748
): boolean {
4849
const existingFactory = registry.getFactory(resolveId);
4950
const matchedExisting = integrations.some(
@@ -59,14 +60,8 @@ export function matchExistingIntegrations<TStrategy, TResolveId extends { [key:
5960
// relying solely on the passed-in strategies.
6061
if (existingFactory && !matchedExisting) {
6162
const message = `A different strategy is registered for ${JSON.stringify(resolveId)}.`;
62-
const { Sentry } = window as SentryWindow;
6363

64-
if (Sentry?.captureMessage) {
65-
Sentry.captureMessage(message);
66-
} else {
67-
// eslint-disable-next-line no-console
68-
console.log(message);
69-
}
64+
errorLogger.log(new Error(message));
7065
}
7166

7267
return matchedExisting;

‎packages/core/src/payment/create-payment-strategy-registry.spec.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createRequestSender } from '@bigcommerce/request-sender';
33
import { createScriptLoader } from '@bigcommerce/script-loader';
44

55
import { createCheckoutStore } from '../checkout';
6+
import { ErrorLogger } from '../common/error';
67
import { createSpamProtection } from '../spam-protection';
78

89
import createPaymentStrategyRegistry from './create-payment-strategy-registry';
@@ -26,13 +27,15 @@ describe('CreatePaymentStrategyRegistry', () => {
2627
const requestSender = createRequestSender();
2728
const paymentClient = createPaymentClient();
2829
const spamProtection = createSpamProtection(createScriptLoader());
30+
const errorLogger: ErrorLogger = { log: jest.fn() };
2931

3032
registry = createPaymentStrategyRegistry(
3133
store,
3234
paymentClient,
3335
requestSender,
3436
spamProtection,
3537
'en_US',
38+
errorLogger,
3639
);
3740
});
3841

0 commit comments

Comments
 (0)