-
Notifications
You must be signed in to change notification settings - Fork 141
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
add graceful shutdown #604
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
2c365af
add graceful shutdown
silesky 745922f
tweak README
silesky 69dd1f4
don't await ready
silesky ec8c119
respect the drained delay
silesky 69a5db7
make less brittle
silesky b5df7e7
rename test file
silesky 21b2ea0
tweak docs
silesky 5686250
rename variable
silesky dec2a8f
tweak
silesky efb08ac
get rid of drained delay because not needed for now
silesky 2035675
update README
silesky 7cd3356
update README
silesky e695bf8
delete unused helper
silesky 3eb0d3c
update README
silesky 307cc96
update README
silesky 8d59e3f
update README
silesky cd04a0b
oops
silesky fbc5f3c
Update README.md
silesky 4614ebf
Update graceful-shutdown-integration.test.ts
silesky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/** @type { import('eslint').Linter.Config } */ | ||
module.exports = { | ||
extends: ['../../.eslintrc'], | ||
env: { | ||
node: true, | ||
}, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = require("@internal/config").lintStagedConfig |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# This is for code that will used as part of testing | ||
|
||
There is no build step included because we use ts-jest, so this could gets compiled in-memory. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
const { createJestTSConfig } = require('@internal/config') | ||
|
||
module.exports = createJestTSConfig() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
{ | ||
"name": "@internal/test-helpers", | ||
"version": "0.0.0", | ||
"private": true, | ||
"engines": { | ||
"node": ">=12" | ||
}, | ||
"scripts": { | ||
"lint": "yarn concurrently 'yarn:eslint .' 'yarn:tsc --noEmit'", | ||
"tsc": "yarn run -T tsc", | ||
"eslint": "yarn run -T eslint", | ||
"concurrently": "yarn run -T concurrently" | ||
}, | ||
"dependencies": { | ||
"tslib": "^2.4.0" | ||
}, | ||
"packageManager": "[email protected]", | ||
"devDependencies": { | ||
"@internal/config": "0.0.0" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"extends": "../../tsconfig.json", | ||
"exclude": ["node_modules", "dist"], | ||
"compilerOptions": { | ||
"resolveJsonModule": true, | ||
"module": "esnext", | ||
"target": "ES5", | ||
"moduleResolution": "node", | ||
"lib": ["es2020"] | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
165 changes: 165 additions & 0 deletions
165
packages/node/src/__tests__/graceful-shutdown-integration.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,165 @@ | ||
import { createSuccess } from './test-helpers/factories' | ||
|
||
const fetcher = jest.fn().mockReturnValue(createSuccess()) | ||
jest.mock('node-fetch', () => fetcher) | ||
|
||
import { AnalyticsNode, NodeSegmentEvent } from '../app/analytics-node' | ||
import { sleep } from './test-helpers/sleep' | ||
import { CoreContext, CorePlugin } from '@segment/analytics-core' | ||
|
||
const testPlugin: CorePlugin = { | ||
type: 'after', | ||
load: () => Promise.resolve(), | ||
name: 'foo', | ||
version: 'bar', | ||
isLoaded: () => true, | ||
} | ||
|
||
describe('Ability for users to exit without losing events', () => { | ||
let ajs!: AnalyticsNode | ||
beforeEach(async () => { | ||
jest.resetAllMocks() | ||
ajs = new AnalyticsNode({ | ||
writeKey: 'abc123', | ||
}) | ||
}) | ||
const _helpers = { | ||
makeTrackCall: (analytics = ajs, cb?: (...args: any[]) => void) => { | ||
analytics.track({ userId: 'foo', event: 'Thing Updated', callback: cb }) | ||
}, | ||
} | ||
|
||
describe('drained emitted event', () => { | ||
test('emits a drained event if only one event is dispatched', async () => { | ||
_helpers.makeTrackCall() | ||
return expect( | ||
new Promise((resolve) => ajs.once('drained', () => resolve(undefined))) | ||
).resolves.toBe(undefined) | ||
}) | ||
|
||
test('emits a drained event if multiple events are dispatched', async () => { | ||
let drainedCalls = 0 | ||
ajs.on('drained', () => { | ||
drainedCalls++ | ||
}) | ||
_helpers.makeTrackCall() | ||
_helpers.makeTrackCall() | ||
_helpers.makeTrackCall() | ||
await sleep(200) | ||
expect(drainedCalls).toBe(1) | ||
}) | ||
|
||
test('all callbacks should be called ', async () => { | ||
const cb = jest.fn() | ||
ajs.track({ userId: 'foo', event: 'bar', callback: cb }) | ||
expect(cb).not.toHaveBeenCalled() | ||
await ajs.closeAndFlush() | ||
expect(cb).toBeCalled() | ||
}) | ||
|
||
test('all async callbacks should be called', async () => { | ||
const trackCall = new Promise<CoreContext>((resolve) => | ||
ajs.track({ | ||
userId: 'abc', | ||
event: 'def', | ||
callback: (ctx) => { | ||
return sleep(100).then(() => resolve(ctx)) | ||
}, | ||
}) | ||
) | ||
const res = await Promise.race([ajs.closeAndFlush(), trackCall]) | ||
expect(res instanceof CoreContext).toBe(true) | ||
}) | ||
}) | ||
|
||
describe('.closeAndFlush()', () => { | ||
test('should force resolve if method call execution time exceeds specified timeout', async () => { | ||
const TIMEOUT = 300 | ||
await ajs.register({ | ||
...testPlugin, | ||
track: async (ctx) => { | ||
await sleep(1000) | ||
return ctx | ||
}, | ||
}) | ||
_helpers.makeTrackCall(ajs) | ||
const startTime = Date.now() | ||
await ajs.closeAndFlush({ timeout: TIMEOUT }) | ||
const elapsedTime = Math.round(Date.now() - startTime) | ||
expect(elapsedTime).toBeLessThanOrEqual(TIMEOUT + 10) | ||
expect(elapsedTime).toBeGreaterThan(TIMEOUT - 10) | ||
}) | ||
|
||
test('no new events should be accepted (but existing ones should be flushed)', async () => { | ||
let trackCallCount = 0 | ||
ajs.on('track', () => { | ||
// track should only happen after successful dispatch | ||
trackCallCount += 1 | ||
}) | ||
_helpers.makeTrackCall() | ||
const closed = ajs.closeAndFlush() | ||
_helpers.makeTrackCall() // should not trigger | ||
_helpers.makeTrackCall() // should not trigger | ||
await closed | ||
expect(fetcher).toBeCalledTimes(1) | ||
expect(trackCallCount).toBe(1) | ||
}) | ||
test('any events created after close should be emitted', async () => { | ||
const events: NodeSegmentEvent[] = [] | ||
ajs.on('call_after_close', (event) => { | ||
events.push(event) | ||
}) | ||
_helpers.makeTrackCall() | ||
const closed = ajs.closeAndFlush() | ||
_helpers.makeTrackCall() // should be emitted | ||
_helpers.makeTrackCall() // should be emitted | ||
expect(events.length).toBe(2) | ||
expect(events.every((e) => e.type === 'track')).toBeTruthy() | ||
await closed | ||
}) | ||
|
||
test('if queue has multiple track events, all of those items should be dispatched, and drain and track events should be emitted', async () => { | ||
let drainedCalls = 0 | ||
ajs.on('drained', () => { | ||
drainedCalls++ | ||
}) | ||
let trackCalls = 0 | ||
ajs.on('track', () => { | ||
trackCalls++ | ||
}) | ||
await ajs.register({ | ||
...testPlugin, | ||
track: async (ctx) => { | ||
await sleep(300) | ||
return ctx | ||
}, | ||
}) | ||
_helpers.makeTrackCall() | ||
_helpers.makeTrackCall() | ||
|
||
await ajs.closeAndFlush() | ||
|
||
expect(fetcher.mock.calls.length).toBe(2) | ||
|
||
expect(trackCalls).toBe(2) | ||
|
||
expect(drainedCalls).toBe(1) | ||
}) | ||
|
||
test('if no pending events, resolves immediately', async () => { | ||
const startTime = Date.now() | ||
await ajs.closeAndFlush() | ||
const elapsedTime = startTime - Date.now() | ||
expect(elapsedTime).toBeLessThan(20) | ||
}) | ||
|
||
test('if no pending events, drained should not be emitted an extra time when close is called', async () => { | ||
let called = false | ||
ajs.on('drained', () => { | ||
called = true | ||
}) | ||
await ajs.closeAndFlush() | ||
expect(called).toBeFalsy() | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export function sleep(timeoutInMs: number): Promise<void> { | ||
return new Promise((resolve) => setTimeout(resolve, timeoutInMs)) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, didn't realize we had
sleep
defined here too. In the browser package we had it inlib/sleep.ts
as well so wouldn't need it defined it its version of this core file like we do currently 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we define sleep everywhere!
I figured we would make core have the canonical version and remove the other ones — thoughts? Or maybe we create a utils package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good to me!