fix: fixing scheduling of flush intervals (segmentio#457)

* fix: fixing scheduling of flush intervals

* fix: use fake timers on flush tests

Author: oscb

README.md

@@ -113,9 +113,7 @@ You must pass at least the `writeKey`. Additional configuration options are list
 | `debug`                    | true\*    | When set to false, it will not generate any logs.                                                                                    |
 | `flushAt`                  | 20        | How many events to accumulate before sending events to the backend.                                                                  |
 | `flushInterval`            | 30        | In seconds, how often to send events to the backend.                                                                                 |
-| `retryInterval`            | 60        | In seconds, how often to to retry sending events the request failed (e.g. in case of a network failure).                             |
 | `maxBatchSize`             | 1000      | How many events to send to the API at once                                                                                           |
-| `maxEventsToRetry`         | 1000      | How many events to keep around for to retry sending if the initial request failed                                                    |
 | `trackAppLifecycleEvents`  | false     | Enable automatic tracking for [app lifecycle events](https://segment.com/docs/connections/spec/mobile/#lifecycle-events): application installed, opened, updated, backgrounded) |
 | `trackDeepLinks`           | false     | Enable automatic tracking for when the user opens the app via a deep link (Note: Requires additional setup on iOS, [see instructions](#ios-deep-link-tracking-setup))                                                            |
 | `defaultSettings`          | undefined | Settings that will be used if the request to get the settings from Segment fails                                                     |

packages/core/src/__tests__/analytics.test.ts

@@ -14,7 +14,6 @@ describe('SegmentClient', () => {
     config: {
       writeKey: 'SEGMENT_KEY',
       flushAt: 10,
-      retryInterval: 40,
       trackAppLifecycleEvents: true,
     },
     logger: getMockLogger(),
@@ -56,8 +55,6 @@ describe('SegmentClient', () => {
       await client.init();
 
       const flush = jest.spyOn(client, 'flush');
-      // An interval should be set to check each second
-      expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 1000);
 
       // Wait 10 secs for the flush interval to happen
       jest.advanceTimersByTime(10 * 1000);
@@ -143,7 +140,6 @@ describe('SegmentClient onUpdateStore', () => {
     config: {
       writeKey: 'SEGMENT_KEY',
       flushAt: 10,
-      retryInterval: 40,
       trackAppLifecycleEvents: true,
     },
     logger: getMockLogger(),
@@ -182,7 +178,6 @@ describe('SegmentClient onUpdateStore', () => {
       config: {
         ...clientArgs.config,
         flushAt,
-        retryInterval: 1,
       },
     };
     const segmentClient = new SegmentClient(args);

packages/core/src/__tests__/methods/flush.test.ts

@@ -25,6 +25,9 @@ describe('methods #flush', () => {
     logger: getMockLogger(),
     store: store,
   };
+  beforeEach(() => {
+    jest.useFakeTimers();
+  });
 
   afterEach(() => {
     store.reset();

packages/core/src/analytics.ts

@@ -39,9 +39,6 @@ export class SegmentClient {
   // Storage
   private store: Storage;
 
-  // how many seconds has elapsed since the last time events were sent
-  private secondsElapsed: number = 0;
-
   // current app state
   private appState: AppStateStatus | 'unknown' = 'unknown';
 
@@ -221,7 +218,7 @@ export class SegmentClient {
     await this.fetchSettings();
 
     // flush any stored events
-    this.flush();
+    this.flush(false);
 
     // set up the timer/subscription for knowing when to flush events
     this.setupInterval();
@@ -304,7 +301,10 @@ export class SegmentClient {
     if (this.flushInterval !== null && this.flushInterval !== undefined) {
       clearInterval(this.flushInterval);
     }
-    this.flushInterval = setInterval(() => this.tick(), 1000) as any;
+
+    this.flushInterval = setTimeout(() => {
+      this.flush();
+    }, this.config.flushInterval! * 1000);
   }
 
   private setupStorageSubscribers() {
@@ -445,23 +445,19 @@ export class SegmentClient {
     }
   }
 
-  private tick() {
-    if (this.secondsElapsed + 1 >= this.config.flushInterval!) {
-      this.flush();
-    } else {
-      this.secondsElapsed += 1;
+  async flush(debounceInterval: boolean = true) {
+    if (this.destroyed) {
+      return;
+    }
+
+    if (debounceInterval) {
+      // Reset interval
+      this.setupInterval();
     }
-  }
 
-  async flush() {
     if (!this.isPendingUpload) {
       this.isPendingUpload = true;
       try {
-        if (this.destroyed) {
-          return;
-        }
-
-        this.secondsElapsed = 0;
         const events = this.store.events.get();
 
         if (events.length > 0) {

packages/core/src/constants.e2e.mock.ts

@@ -12,9 +12,7 @@ export const defaultConfig: Config = {
   writeKey: '',
   flushAt: 20,
   flushInterval: 30,
-  retryInterval: 60,
   maxBatchSize: 1000,
-  maxEventsToRetry: 1000,
   trackDeepLinks: false,
   trackAppLifecycleEvents: false,
   autoAddSegmentDestination: true,

packages/core/src/constants.ts

@@ -7,9 +7,7 @@ export const defaultConfig: Config = {
   writeKey: '',
   flushAt: 20,
   flushInterval: 30,
-  retryInterval: 60,
   maxBatchSize: 1000,
-  maxEventsToRetry: 1000,
   trackDeepLinks: false,
   trackAppLifecycleEvents: false,
   autoAddSegmentDestination: true,

packages/core/src/plugins/SegmentDestination.ts

@@ -57,7 +57,7 @@ export class SegmentDestination extends DestinationPlugin {
       this.analytics?.getConfig().maxBatchSize ?? MAX_EVENTS_PER_BATCH
     );
 
-    let sentEvents: any[] = [];
+    let sentEvents: SegmentEvent[] = [];
     let numFailedEvents = 0;
 
     await Promise.all(

packages/core/src/types.ts

@@ -117,10 +117,8 @@ export type Config = {
   flushAt?: number;
   flushInterval?: number;
   trackAppLifecycleEvents?: boolean;
-  retryInterval?: number;
   maxBatchSize?: number;
   trackDeepLinks?: boolean;
-  maxEventsToRetry?: number;
   defaultSettings?: SegmentAPISettings;
   autoAddSegmentDestination?: boolean;
   collectDeviceId?: boolean;