Skip to content

fix: fixing scheduling of flush intervals #457

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 2 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
5 changes: 0 additions & 5 deletions packages/core/src/__tests__/analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('SegmentClient', () => {
config: {
writeKey: 'SEGMENT_KEY',
flushAt: 10,
retryInterval: 40,
trackAppLifecycleEvents: true,
},
logger: getMockLogger(),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -143,7 +140,6 @@ describe('SegmentClient onUpdateStore', () => {
config: {
writeKey: 'SEGMENT_KEY',
flushAt: 10,
retryInterval: 40,
trackAppLifecycleEvents: true,
},
logger: getMockLogger(),
Expand Down Expand Up @@ -182,7 +178,6 @@ describe('SegmentClient onUpdateStore', () => {
config: {
...clientArgs.config,
flushAt,
retryInterval: 1,
},
};
const segmentClient = new SegmentClient(args);
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/__tests__/methods/flush.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ describe('methods #flush', () => {
logger: getMockLogger(),
store: store,
};
beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
store.reset();
Expand Down
30 changes: 13 additions & 17 deletions packages/core/src/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/constants.e2e.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/plugins/SegmentDestination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down