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: fixed retrying logic for invalid sdk key #978

Merged
merged 1 commit into from
Nov 25, 2024
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
28 changes: 27 additions & 1 deletion sdk/nodejs/__tests__/eventQueue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import { publishEvents } from '../src/request'

import testData from '@devcycle/bucketing-test-data/json-data/testData.json'
import { ResponseError } from '@devcycle/server-request'
const { config } = testData

const publishEvents_mock = mocked(publishEvents)
Expand Down Expand Up @@ -54,7 +55,7 @@
},
},
}
const mockFetchResponse = (obj: any): Response =>

Check warning on line 58 in sdk/nodejs/__tests__/eventQueue.test.ts

View workflow job for this annotation

GitHub Actions / build (20.x)

Unexpected any. Specify a different type
new Response('{}', {
status: 200,
statusText: '',
Expand Down Expand Up @@ -231,7 +232,6 @@

const promise1 = eventQueue.flushEvents()
const promise2 = eventQueue.flushEvents()
eventQueue.cleanup()

await Promise.all([promise1, promise2])
expect(publishEvents_mock).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -327,6 +327,7 @@
],
'localhost:3000/client/1',
)
eventQueue.cleanup()
})

it(
Expand Down Expand Up @@ -816,6 +817,31 @@
},
)

it('should close the event queue if the events api response is 401', async () => {
const eventQueue = initEventQueue('sdkKey', 'uuid')
const error = new ResponseError('401 error')
error.status = 401
publishEvents_mock.mockRejectedValueOnce(error)
eventQueue.queueEvent(
DVCPopulatedUserFromDevCycleUser({ user_id: 'user1' }),
{ type: 'test_event' },
)
await eventQueue.flushEvents()

expect(eventQueue.disabledEventFlush).toBe(true)

eventQueue.queueEvent(
DVCPopulatedUserFromDevCycleUser({ user_id: 'user1' }),
{ type: 'test_event' },
)
eventQueue.queueAggregateEvent(
DVCPopulatedUserFromDevCycleUser({ user_id: 'user1' }),
{ type: EventTypes.aggVariableEvaluated, target: 'key' },
bucketedUserConfig,
)
await eventQueue.flushEvents()
})

describe('Max queue size tests', () => {
it(
'should not queue user event if user' +
Expand Down
49 changes: 41 additions & 8 deletions sdk/nodejs/src/eventQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export class EventQueue {
eventFlushIntervalMS: number
flushEventQueueSize: number
maxEventQueueSize: number
disabledEventFlush: boolean
private flushInterval: NodeJS.Timer
private flushInProgress = false
private flushCallbacks: Array<(arg: unknown) => void> = []
Expand All @@ -63,6 +64,8 @@ export class EventQueue {
this.reporter = options.reporter
this.eventsAPIURI = options.eventsAPIURI
this.eventFlushIntervalMS = options?.eventFlushIntervalMS || 10 * 1000
this.disabledEventFlush = false

if (this.eventFlushIntervalMS < 500) {
throw new Error(
`eventFlushIntervalMS: ${this.eventFlushIntervalMS} must be larger than 500ms`,
Expand Down Expand Up @@ -121,6 +124,7 @@ export class EventQueue {

cleanup(): void {
clearInterval(this.flushInterval)
this.disabledEventFlush = true
}

private async _flushEvents() {
Expand Down Expand Up @@ -219,12 +223,25 @@ export class EventQueue {
this.logger.debug(
`DevCycle Error Flushing Events response message: ${ex.message}`,
)
this.bucketing.onPayloadFailure(
this.sdkKey,
flushPayload.payloadId,
true,
)
results.retries++
if ('status' in ex && ex.status === 401) {
this.logger.debug(
`SDK key is invalid, closing event flushing interval`,
)
this.bucketing.onPayloadFailure(
this.sdkKey,
flushPayload.payloadId,
false,
)
results.failures++
this.cleanup()
} else {
this.bucketing.onPayloadFailure(
this.sdkKey,
flushPayload.payloadId,
true,
)
results.retries++
}
}
}),
)
Expand Down Expand Up @@ -276,9 +293,19 @@ export class EventQueue {
* Queue DVCAPIEvent for publishing to DevCycle Events API.
*/
queueEvent(user: DVCPopulatedUser, event: DevCycleEvent): void {
if (this.disabledEventFlush) {
this.logger.warn(
`Event flushing is disabled, dropping event: ${
event.type
}, event queue size: ${this.bucketing.eventQueueSize(
this.sdkKey,
)}`,
)
return
}
if (this.checkEventQueueSize()) {
this.logger.warn(
`Max event queue size reached, dropping event: ${event}`,
`Max event queue size reached, dropping event: ${event.type}`,
)
return
}
Expand All @@ -299,9 +326,15 @@ export class EventQueue {
event: DevCycleEvent,
bucketedConfig?: BucketedUserConfig,
): void {
if (this.disabledEventFlush) {
this.logger.warn(
`Event flushing is disabled, dropping aggregate event: ${event.type}`,
)
return
}
if (this.checkEventQueueSize()) {
this.logger.warn(
`Max event queue size reached, dropping aggregate event: ${event}`,
`Max event queue size reached, dropping aggregate event: ${event.type}`,
)
return
}
Expand Down
Loading