Skip to content
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

fix: shutdownAsync flushes the whole queue #191

Merged
merged 16 commits into from
Feb 26, 2024
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@
"eslint-plugin-react": "^7.30.1",
"eslint-plugin-react-hooks": "^4.6.0",
"husky": "^8.0.1",
"jest": "^28.1.3",
"jest": "^29.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relates to jestjs/jest#14741 (comment) otherwise I cannot run tests locally

"jest-environment-jsdom": "^28.1.3",
"prettier": "^2.7.1",
"rollup": "^2.77.0",
"rollup-plugin-dts": "^4.2.2",
"rollup-plugin-dts-bundle": "^1.0.0",
"rollup-plugin-typescript2": "^0.32.1",
"ts-jest": "^28.0.6",
"ts-jest": "^29.0.0",
"ts-node": "^10.9.1",
"tslib": "^2.4.0",
"typescript": "^4.7.4"
Expand Down
33 changes: 25 additions & 8 deletions posthog-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -616,23 +616,40 @@ export abstract class PostHogCoreStateless {
)
}

async shutdownAsync(): Promise<void> {
async shutdownAsync(shutdownTimeoutMs?: number): Promise<void> {
await this._initPromise

clearTimeout(this._flushTimer)
try {
await this.flushAsync()
await Promise.all(
Object.values(this.pendingPromises).map((x) =>
x.catch(() => {
// ignore errors as we are shutting down and can't deal with them anyways.
})
)
)
// flush again to make sure we send all events, some of which might've been added
// while we were waiting for the pending promises to resolve
// For example, see sendFeatureFlags in posthog-node/src/posthog-node.ts::capture
await this.flushAsync()

const timeout = shutdownTimeoutMs ?? 30000
const startTimeWithDelay = Date.now() + timeout

while (true) {
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
const queue = this.getPersistedProperty<PostHogQueueItem[]>(PostHogPersistedProperty.Queue) || []

if (queue.length === 0) {
break
}

// flush again to make sure we send all events, some of which might've been added
// while we were waiting for the pending promises to resolve
// For example, see sendFeatureFlags in posthog-node/src/posthog-node.ts::capture
await this.flushAsync()

// If we've been waiting for more than the shutdownTimeoutMs, stop it
const now = Date.now()
if (startTimeWithDelay < now) {
break
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will silently break instead of throwing an error or something since its an intended behavior.

}
}
} catch (e) {
if (!isPostHogFetchError(e)) {
throw e
Expand All @@ -641,8 +658,8 @@ export abstract class PostHogCoreStateless {
}
}

shutdown(): void {
void this.shutdownAsync()
shutdown(shutdownTimeoutMs?: number): void {
void this.shutdownAsync(shutdownTimeoutMs)
}
}

Expand Down
26 changes: 26 additions & 0 deletions posthog-core/test/posthog.shutdown.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { createTestClient, PostHogCoreTestClient, PostHogCoreTestClientMocks } from './test-utils/PostHogCoreTestClient'

describe('PostHog Core', () => {
let posthog: PostHogCoreTestClient
let mocks: PostHogCoreTestClientMocks

describe('shutdown', () => {
beforeEach(() => {
jest.useRealTimers()
;[posthog, mocks] = createTestClient('TEST_API_KEY', {
flushAt: 10,
preloadFeatureFlags: false,
captureMode: 'json',
})
})

it('flush messsages once called', async () => {
for (let i = 0; i < 5; i++) {
posthog.capture('test-event')
}

await posthog.shutdownAsync()
expect(mocks.fetch).toHaveBeenCalledTimes(1)
})
})
})
1 change: 1 addition & 0 deletions posthog-node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Adds a `disabled` option and the ability to change it later via `posthog.disabled = true`. Useful for disabling PostHog tracking for example in a testing environment without having complex conditional checking
- Fixes some typos in types
- `shutdown` and `shutdownAsync` takes a `shutdownTimeoutMs` param with a default of 30000 (30s). This is the time to wait for flushing events before shutting down the client. If the timeout is reached, the client will be shut down regardless of pending events.

# 3.6.3 - 2024-02-15

Expand Down
8 changes: 4 additions & 4 deletions posthog-node/src/posthog-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,13 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 {
await this.featureFlagsPoller?.loadFeatureFlags(true)
}

shutdown(): void {
void this.shutdownAsync()
shutdown(shutdownTimeoutMs?: number): void {
void this.shutdownAsync(shutdownTimeoutMs)
}

async shutdownAsync(): Promise<void> {
async shutdownAsync(shutdownTimeoutMs?: number): Promise<void> {
this.featureFlagsPoller?.stopPoller()
return super.shutdownAsync()
return super.shutdownAsync(shutdownTimeoutMs)
}

private addLocalPersonAndGroupProperties(
Expand Down
4 changes: 3 additions & 1 deletion posthog-node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ export type PostHogNodeV1 = {
/**
* @description Flushes the events still in the queue and clears the feature flags poller to allow for
* a clean shutdown.
*
* @param shutdownTimeoutMs The shutdown timeout, in milliseconds. Defaults to 30000 (30s).
*/
shutdown(): void
shutdown(shutdownTimeoutMs?: number): void
}
1 change: 1 addition & 0 deletions posthog-react-native/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Adds a `disabled` option and the ability to change it later via `posthog.disabled = true`. Useful for disabling PostHog tracking for example in a testing environment without having complex conditional checking
- Many methods such as `capture` and `identify` no longer return the `this` object instead returning nothing
- Fixes some typos in types
- `shutdown` and `shutdownAsync` takes a `shutdownTimeoutMs` param with a default of 30000 (30s). This is the time to wait for flushing events before shutting down the client. If the timeout is reached, the client will be shut down regardless of pending events.

# 2.11.6 - 2024-02-22

Expand Down
50 changes: 25 additions & 25 deletions posthog-react-native/test/__snapshots__/autocapture.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -1,88 +1,88 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PostHog React Native autocapture should capture a valid event 1`] = `
Array [
[
"touch",
Array [
Object {
"$el_text": "I have the property \\"ph-label\\" which means touching me will be autocaptured with a specific label",
[
{
"$el_text": "I have the property \"ph-label\" which means touching me will be autocaptured with a specific label",
"attr__ph-label": "special-text",
"attr__testID": "example-ph-label",
"tag_name": "special-text",
},
Object {
"$el_text": "I have the property \\"ph-label\\" which means touching me will be autocaptured with a specific label",
{
"$el_text": "I have the property \"ph-label\" which means touching me will be autocaptured with a specific label",
"attr__testID": "example-ph-label",
"tag_name": "higher-level-element",
},
Object {
{
"attr__style": "backgroundColor:rgba(0,0,0,.1);padding:15;borderRadius:10;marginBottom:10",
"tag_name": "higher-level-element",
},
Object {
{
"attr__ph-label": "higher-level-element",
"tag_name": "higher-level-element",
},
Object {
{
"attr__style": "flex:1",
"tag_name": "ScrollView",
},
Object {
{
"attr__style": "flex:1",
"tag_name": "ScrollView",
},
Object {
{
"tag_name": "PostHogDemoScreen",
},
Object {
{
"tag_name": "StaticContainer",
},
Object {
{
"tag_name": "EnsureSingleNavigator",
},
Object {
{
"tag_name": "SceneView",
},
Object {
{
"attr__style": "flex:1",
"tag_name": "View",
},
Object {
{
"attr__style": "flex:1;backgroundColor:rgb(242, 242, 242);flexDirection:column-reverse",
"tag_name": "View",
},
Object {
{
"attr__style": "flex:1;flexDirection:column-reverse",
"tag_name": "Background",
},
Object {
{
"tag_name": "Screen",
},
Object {
{
"attr__style": "position:absolute;left:0;right:0;top:0;bottom:0;zIndex:0",
"tag_name": "AnimatedComponent",
},
Object {
{
"tag_name": "MaybeFreeze",
},
Object {
{
"attr__style": "position:absolute;left:0;right:0;top:0;bottom:0;zIndex:0",
"tag_name": "Screen",
},
Object {
{
"attr__style": "position:absolute;left:0;right:0;top:0;bottom:0;zIndex:0",
"tag_name": "MaybeScreen",
},
Object {
{
"attr__style": "flex:1;overflow:hidden",
"tag_name": "ScreenContainer",
},
Object {
{
"attr__style": "flex:1;overflow:hidden",
"tag_name": "MaybeScreenContainer",
},
],
Object {
{
"$touch_x": 1,
"$touch_y": 2,
},
Expand Down
1 change: 1 addition & 0 deletions posthog-web/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Adds a `disabled` option and the ability to change it later via `posthog.disabled = true`. Useful for disabling PostHog tracking for example in a testing environment without having complex conditional checking
- Many methods such as `capture` and `identify` no longer return the `this` object instead returning nothing
- Fixes some typos in types
- `shutdown` and `shutdownAsync` takes a `shutdownTimeoutMs` param with a default of 30000 (30s). This is the time to wait for flushing events before shutting down the client. If the timeout is reached, the client will be shut down regardless of pending events.

# 2.6.2 - 2024-02-15

Expand Down
Loading
Loading