Skip to content

Commit 1d64085

Browse files
ashishkeshanheiskr
andauthored
Reduce API Calls to Hydro by Queuing (#55303)
Co-authored-by: Kevin Heis <[email protected]>
1 parent cd42cdb commit 1d64085

File tree

4 files changed

+201
-88
lines changed

4 files changed

+201
-88
lines changed

src/events/components/events.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const COOKIE_NAME = '_docs-events'
1111

1212
const startVisitTime = Date.now()
1313

14+
const BATCH_INTERVAL = 5000 // 5 seconds
15+
1416
let initialized = false
1517
let cookieValue: string | undefined
1618
let pageEventId: string | undefined
@@ -23,6 +25,16 @@ let scrollFlipCount = 0
2325
let maxScrollY = 0
2426
let previousPath: string | undefined
2527
let hoveredUrls = new Set()
28+
let eventQueue: any[] = []
29+
30+
function scheduleNextFlush() {
31+
setTimeout(() => {
32+
flushQueue()
33+
scheduleNextFlush()
34+
}, BATCH_INTERVAL)
35+
}
36+
37+
scheduleNextFlush()
2638

2739
function resetPageParams() {
2840
sentExit = false
@@ -133,17 +145,31 @@ export function sendEvent<T extends EventType>({
133145
...props,
134146
}
135147

136-
const blob = new Blob([JSON.stringify(body)], { type: 'application/json' })
148+
queueEvent(body)
149+
150+
if (type === EventType.exit) {
151+
flushQueue()
152+
}
153+
154+
return body
155+
}
156+
157+
function flushQueue() {
158+
if (!eventQueue.length) return
159+
137160
const endpoint = '/api/events'
161+
const eventsBody = JSON.stringify(eventQueue)
162+
eventQueue = []
163+
138164
try {
139-
// Only send the beacon if the feature is not disabled in the user's browser
140-
// Even if the function exists, it can still throw an error from the call being blocked
141-
navigator?.sendBeacon(endpoint, blob)
142-
} catch {
143-
console.warn(`sendBeacon to '${endpoint}' failed.`)
165+
navigator.sendBeacon(endpoint, new Blob([eventsBody], { type: 'application/json' }))
166+
} catch (err) {
167+
console.warn(`sendBeacon to '${endpoint}' failed.`, err)
144168
}
169+
}
145170

146-
return body
171+
function queueEvent(eventBody: unknown) {
172+
eventQueue.push(eventBody)
147173
}
148174

149175
// Sometimes using the back button means the internal referrer path is not there,
@@ -248,6 +274,8 @@ function initPageAndExitEvent() {
248274
document.addEventListener('visibilitychange', () => {
249275
if (document.visibilityState === 'hidden') {
250276
sendExit()
277+
} else {
278+
flushQueue()
251279
}
252280
})
253281

src/events/middleware.ts

Lines changed: 67 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -43,61 +43,80 @@ router.post(
4343
catchMiddlewareError(async function postEvents(req: ExtendedRequest, res: Response) {
4444
noCacheControl(res)
4545

46-
// Make sure the type is supported before continuing
47-
if (!req.body.type || !allowedTypes.has(req.body.type)) {
48-
return res.status(400).json({ message: 'Invalid type' })
49-
}
50-
const type: EventType = req.body.type
51-
const body: EventProps & EventPropsByType[EventType] = req.body
46+
const eventsToProcess = Array.isArray(req.body) ? req.body : [req.body]
47+
const validEvents: any[] = []
48+
const validationErrors: any[] = []
49+
50+
for (const eventBody of eventsToProcess) {
51+
try {
52+
if (!eventBody.type || !allowedTypes.has(eventBody.type)) {
53+
validationErrors.push({ event: eventBody, error: 'Invalid type' })
54+
continue
55+
}
56+
const type: EventType = eventBody.type
57+
const body: EventProps & EventPropsByType[EventType] = eventBody
58+
if (isSurvey(body) && body.survey_comment) {
59+
body.survey_rating = await getSurveyCommentRating({
60+
comment: body.survey_comment,
61+
language: body.context.path_language || 'en',
62+
})
63+
body.survey_comment_language = await getGuessedLanguage(body.survey_comment)
64+
}
5265

53-
// Validate the data matches the corresponding data schema
54-
const validate = validators[type]
55-
if (!validate(body)) {
56-
// This protects so we don't bother sending the same validation
57-
// error, per user, more than once (per time interval).
58-
// This helps if we're bombarded with junk bot traffic. So it
59-
// protects our Hydro instance from being overloaded with things
60-
// that aren't helping anybody.
61-
const hash = `${req.ip}:${(validate.errors || [])
62-
.map((error: ErrorObject) => error.message + error.instancePath)
63-
.join(':')}`
64-
if (!sentValidationErrors.has(hash)) {
65-
sentValidationErrors.set(hash, true)
66-
// Track validation errors in Hydro so that we can know if
67-
// there's a widespread problem in events.ts
68-
await publish(
69-
formatErrors(validate.errors || [], body).map((error) => ({
70-
schema: hydroNames.validation,
71-
value: error,
72-
})),
73-
)
66+
if (body.context) {
67+
// Add dotcom_user to the context if it's available
68+
// JSON.stringify removes `undefined` values but not `null`, and we don't want to send `null` to Hydro
69+
body.context.dotcom_user = req.cookies?.dotcom_user ? req.cookies.dotcom_user : undefined
70+
body.context.is_staff = Boolean(req.cookies?.staffonly)
71+
}
72+
const validate = validators[type]
73+
if (!validate(body)) {
74+
validationErrors.push({
75+
event: body,
76+
error: validate.errors || [],
77+
})
78+
// This protects so we don't bother sending the same validation
79+
// error, per user, more than once (per time interval).
80+
// This helps if we're bombarded with junk bot traffic. So it
81+
// protects our Hydro instance from being overloaded with things
82+
// that aren't helping anybody.
83+
const hash = `${req.ip}:${(validate.errors || [])
84+
.map((error: ErrorObject) => error.message + error.instancePath)
85+
.join(':')}`
86+
if (!sentValidationErrors.has(hash)) {
87+
sentValidationErrors.set(hash, true)
88+
formatErrors(validate.errors || [], body).map((error) => {
89+
validationErrors.push({ schema: hydroNames.validation, value: error })
90+
})
91+
}
92+
continue
93+
}
94+
validEvents.push({
95+
schema: hydroNames[type],
96+
value: omit(body, OMIT_FIELDS),
97+
})
98+
} catch (eventError) {
99+
console.error('Error validating event:', eventError)
74100
}
75-
// We aren't helping bots spam us :)
76-
return res.status(400).json(isProd ? {} : validate.errors)
77101
}
78-
79-
if (isSurvey(body) && body.survey_comment) {
80-
body.survey_rating = await getSurveyCommentRating({
81-
comment: body.survey_comment,
82-
language: body.context.path_language || 'en',
83-
})
84-
body.survey_comment_language = await getGuessedLanguage(body.survey_comment)
102+
if (validEvents.length > 0) {
103+
await publish(validEvents)
85104
}
86105

87-
// Add dotcom_user to the context if it's available
88-
// JSON.stringify removes `undefined` values but not `null`, and we don't want to send `null` to Hydro
89-
if (body.context) {
90-
body.context.dotcom_user = req.cookies?.dotcom_user ? req.cookies.dotcom_user : undefined
91-
// Add if the user is a staff, using the 'staffonly' cookie
92-
body.context.is_staff = Boolean(req.cookies?.staffonly)
106+
if (validationErrors.length > 0) {
107+
await publish(validationErrors)
93108
}
109+
const statusCode = validationErrors.length > 0 ? 400 : 200
94110

95-
await publish({
96-
schema: hydroNames[type],
97-
value: omit(body, OMIT_FIELDS),
98-
})
99-
100-
return res.json({})
111+
return res.status(statusCode).json(
112+
isProd
113+
? undefined
114+
: {
115+
success_count: validEvents.length,
116+
failure_count: validationErrors.length,
117+
details: validationErrors,
118+
},
119+
)
101120
}),
102121
)
103122

src/events/tests/middleware.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ describe('POST /events', () => {
66
vi.setConfig({ testTimeout: 60 * 1000 })
77

88
async function checkEvent(data: any) {
9+
// if data is not an array, make it one
10+
if (!Array.isArray(data)) {
11+
data = [data]
12+
}
913
const body = JSON.stringify(data)
1014
const res = await post('/api/events', {
1115
body,
@@ -47,15 +51,47 @@ describe('POST /events', () => {
4751
},
4852
}
4953

50-
test('should record a page event', async () => {
51-
const { statusCode } = await checkEvent(pageExample)
54+
const exitExample = {
55+
type: 'exit',
56+
context: {
57+
// Primitives
58+
event_id: 'a35d7f88-3f48-4f36-ad89-5e3c8ebc3df7',
59+
user: '703d32a8-ed0f-45f9-8d78-a913d4dc6f19',
60+
version: '1.0.0',
61+
created: '2020-10-02T17:12:18.620Z',
62+
63+
// Content information
64+
path: '/github/docs/issues',
65+
hostname: 'github.com',
66+
referrer: 'https://github.com/github/docs',
67+
search: '?q=is%3Aissue+is%3Aopen+example+',
68+
href: 'https://github.com/github/docs/issues?q=is%3Aissue+is%3Aopen+example+',
69+
path_language: 'en',
70+
71+
// Device information
72+
os: 'linux',
73+
os_version: '18.04',
74+
browser: 'chrome',
75+
browser_version: '85.0.4183.121',
76+
viewport_width: 1418,
77+
viewport_height: 501,
78+
79+
// Location information
80+
timezone: -7,
81+
user_language: 'en-US',
82+
},
83+
}
84+
85+
test('should record a page and exit event', async () => {
86+
const eventQueue = [pageExample, exitExample]
87+
const { statusCode } = await checkEvent(eventQueue)
5288
expect(statusCode).toBe(200)
5389
})
5490

5591
test('should require a type', async () => {
5692
const { statusCode, body } = await checkEvent({ ...pageExample, type: undefined })
5793
expect(statusCode).toBe(400)
58-
expect(body).toEqual('{"message":"Invalid type"}')
94+
expect(body).toContain('"error":"Invalid type"}')
5995
})
6096

6197
test('should require an event_id in uuid', async () => {

src/fixtures/tests/playwright-rendering.spec.ts

Lines changed: 60 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -638,23 +638,27 @@ test.describe('survey', () => {
638638
// Important to set this up *before* interacting with the page
639639
// in case of possible race conditions.
640640
await page.route('**/api/events', (route, request) => {
641-
route.fulfill({})
642-
expect(request.method()).toBe('POST')
643-
const postData = JSON.parse(request.postData() || '{}')
644-
// Skip the exit event
645-
if (postData.type === 'exit') {
646-
return
647-
}
648-
fulfilled++
649-
if (postData.type === 'survey' && postData.survey_vote === true) {
650-
hasSurveyPressedEvent = true
651-
}
652-
if (
653-
postData.type === 'survey' &&
654-
postData.survey_vote === true &&
655-
postData.survey_comment === surveyComment
656-
) {
657-
hasSurveySubmittedEvent = true
641+
const postData = request.postData()
642+
if (postData) {
643+
const postDataArray = JSON.parse(postData)
644+
route.fulfill({})
645+
expect(request.method()).toBe('POST')
646+
fulfilled = postDataArray.length
647+
for (const eventBody of postDataArray) {
648+
if (eventBody.type === 'survey' && eventBody.survey_vote === true) {
649+
hasSurveyPressedEvent = true
650+
}
651+
if (eventBody.type === 'survey' && eventBody.survey_vote === true) {
652+
hasSurveyPressedEvent = true
653+
}
654+
if (
655+
eventBody.type === 'survey' &&
656+
eventBody.survey_vote === true &&
657+
eventBody.survey_comment === surveyComment
658+
) {
659+
hasSurveySubmittedEvent = true
660+
}
661+
}
658662
}
659663
// At the time of writing you can't get the posted payload
660664
// when you use `navigator.sendBeacon(url, data)`.
@@ -673,16 +677,28 @@ test.describe('survey', () => {
673677
await expect(page.getByRole('button', { name: 'Cancel' })).toBeVisible()
674678
await expect(page.getByRole('button', { name: 'Send' })).toBeVisible()
675679

676-
await page.locator('[for=survey-comment]').click()
677680
await page.locator('[for=survey-comment]').fill(surveyComment)
678681
await page.locator('[name=survey-email]').click()
679682
await page.locator('[name=survey-email]').fill('[email protected]')
680683
await page.getByRole('button', { name: 'Send' }).click()
684+
// simulate sending an exit event to trigger sending all queued events
685+
await page.evaluate(() => {
686+
Object.defineProperty(document, 'visibilityState', {
687+
configurable: true,
688+
get: function () {
689+
return 'hidden'
690+
},
691+
})
692+
document.dispatchEvent(new Event('visibilitychange'))
693+
return new Promise((resolve) => setTimeout(resolve, 100))
694+
})
695+
681696
// Events:
682697
// 1. page view event when navigating to the page
683698
// 2. Survey thumbs up event
684699
// 3. Survey submit event
685-
expect(fulfilled).toBe(1 + 1 + 1)
700+
// 4. Exit event
701+
expect(fulfilled).toBe(1 + 1 + 1 + 1)
686702
expect(hasSurveyPressedEvent).toBe(true)
687703
expect(hasSurveySubmittedEvent).toBe(true)
688704
await expect(page.getByTestId('survey-end')).toBeVisible()
@@ -691,20 +707,22 @@ test.describe('survey', () => {
691707
test('thumbs up without filling in the form sends an API POST', async ({ page }) => {
692708
let fulfilled = 0
693709
let hasSurveyEvent = false
710+
694711
// Important to set this up *before* interacting with the page
695712
// in case of possible race conditions.
696713
await page.route('**/api/events', (route, request) => {
697-
route.fulfill({})
698-
expect(request.method()).toBe('POST')
699-
const postData = JSON.parse(request.postData() || '{}')
700-
// Skip the exit event
701-
if (postData.type === 'exit') {
702-
return
703-
}
704-
if (postData.type === 'survey' && postData.survey_vote === true) {
705-
hasSurveyEvent = true
714+
const postData = request.postData()
715+
if (postData) {
716+
const postDataArray = JSON.parse(postData)
717+
route.fulfill({})
718+
expect(request.method()).toBe('POST')
719+
fulfilled = postDataArray.length
720+
for (const eventBody of postDataArray) {
721+
if (eventBody.type === 'survey' && eventBody.survey_vote === true) {
722+
hasSurveyEvent = true
723+
}
724+
}
706725
}
707-
fulfilled++
708726
// At the time of writing you can't get the posted payload
709727
// when you use `navigator.sendBeacon(url, data)`.
710728
// So we can't make assertions about the payload.
@@ -718,10 +736,22 @@ test.describe('survey', () => {
718736
await page.goto('/get-started/foo/for-playwright')
719737

720738
await page.locator('[for=survey-yes]').click()
739+
// simulate sending an exit event to trigger sending all queued events
740+
await page.evaluate(() => {
741+
Object.defineProperty(document, 'visibilityState', {
742+
configurable: true,
743+
get: function () {
744+
return 'hidden'
745+
},
746+
})
747+
document.dispatchEvent(new Event('visibilitychange'))
748+
return new Promise((resolve) => setTimeout(resolve, 100))
749+
})
721750
// Events:
722751
// 1. page view event when navigating to the page
723752
// 2. the thumbs up click
724-
expect(fulfilled).toBe(1 + 1)
753+
// 3. the exit event
754+
expect(fulfilled).toBe(1 + 1 + 1)
725755
expect(hasSurveyEvent).toBe(true)
726756

727757
await expect(page.getByRole('button', { name: 'Send' })).toBeVisible()

0 commit comments

Comments
 (0)