fix: Log errors when there is no error event handler. (#495)

When there are no error event handlers the server SDKs should log the
errors instead.

This behavior was missed during the typescript re-write.

We may want to consider if we always want to log errors, but I
understand the logic of this. If you are listening to this, then you may
also be logging it, and if you are not logging it, then you are at least
getting the event.

I imagine most consumers are not using the error events.

Author: kinyoklion

packages/sdk/server-node/__tests__/LDClientNode.listeners.test.ts

@@ -1,6 +1,7 @@
 import { integrations } from '@launchdarkly/js-server-sdk-common';
+import { logger } from '@launchdarkly/private-js-mocks';
 
-import { LDClient } from '../src';
+import { Context, LDClient } from '../src';
 import LDClientNode from '../src/LDClientNode';
 
 describe('given an LDClient with test data', () => {
@@ -12,6 +13,7 @@ describe('given an LDClient with test data', () => {
     client = new LDClientNode('sdk-key', {
       updateProcessor: td.getFactory(),
       sendEvents: false,
+      logger,
     });
   });
 
@@ -78,4 +80,16 @@ describe('given an LDClient with test data', () => {
     td.update(td.flag('flag1').on(false));
     td.update(td.flag('flag2').on(false));
   });
+
+  it('logs errors when there are no event handlers', () => {
+    // Empty kind is not valid.
+    const invalidContext = { key: 'key', kind: '' };
+    client.variation('flag', { key: 'key', kind: '' }, false);
+    const referenceContext = Context.fromLDContext(invalidContext);
+    expect(referenceContext.message).toBeDefined();
+    expect(logger.error).toHaveBeenNthCalledWith(
+      1,
+      `${referenceContext.message} returning default value.`,
+    );
+  });
 });

packages/sdk/server-node/src/LDClientNode.ts

@@ -43,6 +43,8 @@ class LDClientNode extends LDClientImpl {
         onError: (err: Error) => {
           if (emitter.listenerCount('error')) {
             emitter.emit('error', err);
+          } else {
+            logger.error(err.message);
           }
         },
         onFailed: (err: Error) => {

packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts

@@ -29,7 +29,8 @@ class LDClient extends LDClientImpl {
     options: LDOptions,
     storeProvider: CacheableStoreProvider,
   ) {
-    super(sdkKey, platform, createOptions(options), createCallbacks());
+    const finalOptions = createOptions(options);
+    super(sdkKey, platform, finalOptions, createCallbacks(finalOptions.logger));
     this.cacheableStoreProvider = storeProvider;
   }
 

packages/shared/akamai-edgeworker-sdk/src/utils/createCallbacks.ts

@@ -1,6 +1,10 @@
+import { LDLogger } from '@launchdarkly/js-server-sdk-common';
+
 // eslint-disable-next-line import/prefer-default-export
-export const createCallbacks = () => ({
-  onError: (_err: Error) => {},
+export const createCallbacks = (logger?: LDLogger) => ({
+  onError: (err: Error) => {
+    logger?.error?.(err.message);
+  },
   onFailed: (_err: Error) => {},
   onReady: () => {},
   onUpdate: (_key: string) => {},

packages/shared/sdk-server-edge/src/api/LDClient.ts

@@ -21,7 +21,16 @@ export default class LDClient extends LDClientImpl {
       diagnosticEventPath: `/events/diagnostic/${clientSideID}`,
       includeAuthorizationHeader: false,
     };
-    super(clientSideID, platform, createOptions(options), createCallbacks(em), internalOptions);
+
+    const finalOptions = createOptions(options);
+
+    super(
+      clientSideID,
+      platform,
+      finalOptions,
+      createCallbacks(em, finalOptions.logger),
+      internalOptions,
+    );
     this.emitter = em;
   }
 }

packages/shared/sdk-server-edge/src/api/createCallbacks.test.ts

@@ -1,6 +1,7 @@
 import { EventEmitter } from 'node:events';
 
 import { noop } from '@launchdarkly/js-server-sdk-common';
+import { logger } from '@launchdarkly/private-js-mocks';
 
 import createCallbacks from './createCallbacks';
 
@@ -17,7 +18,7 @@ describe('createCallbacks', () => {
     jest.resetAllMocks();
   });
 
-  test('onError', () => {
+  test('onError calls the emitter', () => {
     emitter.on('error', noop);
 
     const { onError } = createCallbacks(emitter);
@@ -26,6 +27,29 @@ describe('createCallbacks', () => {
     expect(emitter.emit).toHaveBeenNthCalledWith(1, 'error', err);
   });
 
+  test('onError does not log when there are listeners', () => {
+    emitter.on('error', noop);
+
+    const { onError } = createCallbacks(emitter, logger);
+    onError(err);
+
+    expect(emitter.emit).toHaveBeenNthCalledWith(1, 'error', err);
+    expect(logger.error).not.toHaveBeenCalled();
+    expect(logger.warn).not.toHaveBeenCalled();
+    expect(logger.info).not.toHaveBeenCalled();
+    expect(logger.debug).not.toHaveBeenCalled();
+  });
+
+  test('onError logs when there are no listeners', () => {
+    const { onError } = createCallbacks(emitter, logger);
+    onError(err);
+
+    expect(logger.error).toHaveBeenNthCalledWith(1, err.message);
+    expect(logger.warn).not.toHaveBeenCalled();
+    expect(logger.info).not.toHaveBeenCalled();
+    expect(logger.debug).not.toHaveBeenCalled();
+  });
+
   test('onError should not be called', () => {
     const { onError } = createCallbacks(emitter);
     onError(err);

packages/shared/sdk-server-edge/src/api/createCallbacks.ts

@@ -1,11 +1,13 @@
 import { EventEmitter } from 'node:events';
 
-import { noop } from '@launchdarkly/js-server-sdk-common';
+import { LDLogger, noop } from '@launchdarkly/js-server-sdk-common';
 
-const createCallbacks = (emitter: EventEmitter) => ({
+const createCallbacks = (emitter: EventEmitter, logger?: LDLogger) => ({
   onError: (err: Error) => {
     if (emitter.listenerCount('error')) {
       emitter.emit('error', err);
+    } else {
+      logger?.error(err.message);
     }
   },
   onFailed: (err: Error) => {