diff --git a/data/features/security-overview-push-protection-metrics-page.yml b/data/features/security-overview-push-protection-metrics-page.yml deleted file mode 100644 index 23f9cb287f94..000000000000 --- a/data/features/security-overview-push-protection-metrics-page.yml +++ /dev/null @@ -1,7 +0,0 @@ -# Reference: #9141. -# Security overview - secret scanning push protection metrics -# Ref 17108 Advanced Security available to Team plans -versions: - fpt: '*' - ghec: '*' - ghes: '> 3.10' diff --git a/src/events/components/events.ts b/src/events/components/events.ts index ab0861c71d94..bebb5c272ace 100644 --- a/src/events/components/events.ts +++ b/src/events/components/events.ts @@ -11,6 +11,8 @@ const COOKIE_NAME = '_docs-events' const startVisitTime = Date.now() +const BATCH_INTERVAL = 5000 // 5 seconds + let initialized = false let cookieValue: string | undefined let pageEventId: string | undefined @@ -23,6 +25,16 @@ let scrollFlipCount = 0 let maxScrollY = 0 let previousPath: string | undefined let hoveredUrls = new Set() +let eventQueue: any[] = [] + +function scheduleNextFlush() { + setTimeout(() => { + flushQueue() + scheduleNextFlush() + }, BATCH_INTERVAL) +} + +scheduleNextFlush() function resetPageParams() { sentExit = false @@ -133,17 +145,31 @@ export function sendEvent({ ...props, } - const blob = new Blob([JSON.stringify(body)], { type: 'application/json' }) + queueEvent(body) + + if (type === EventType.exit) { + flushQueue() + } + + return body +} + +function flushQueue() { + if (!eventQueue.length) return + const endpoint = '/api/events' + const eventsBody = JSON.stringify(eventQueue) + eventQueue = [] + try { - // Only send the beacon if the feature is not disabled in the user's browser - // Even if the function exists, it can still throw an error from the call being blocked - navigator?.sendBeacon(endpoint, blob) - } catch { - console.warn(`sendBeacon to '${endpoint}' failed.`) + navigator.sendBeacon(endpoint, new Blob([eventsBody], { type: 'application/json' })) + } catch (err) { + console.warn(`sendBeacon to '${endpoint}' failed.`, err) } +} - return body +function queueEvent(eventBody: unknown) { + eventQueue.push(eventBody) } // Sometimes using the back button means the internal referrer path is not there, @@ -248,6 +274,8 @@ function initPageAndExitEvent() { document.addEventListener('visibilitychange', () => { if (document.visibilityState === 'hidden') { sendExit() + } else { + flushQueue() } }) diff --git a/src/events/middleware.ts b/src/events/middleware.ts index 89635f35ca92..2b1210c1b268 100644 --- a/src/events/middleware.ts +++ b/src/events/middleware.ts @@ -43,61 +43,80 @@ router.post( catchMiddlewareError(async function postEvents(req: ExtendedRequest, res: Response) { noCacheControl(res) - // Make sure the type is supported before continuing - if (!req.body.type || !allowedTypes.has(req.body.type)) { - return res.status(400).json({ message: 'Invalid type' }) - } - const type: EventType = req.body.type - const body: EventProps & EventPropsByType[EventType] = req.body + const eventsToProcess = Array.isArray(req.body) ? req.body : [req.body] + const validEvents: any[] = [] + const validationErrors: any[] = [] + + for (const eventBody of eventsToProcess) { + try { + if (!eventBody.type || !allowedTypes.has(eventBody.type)) { + validationErrors.push({ event: eventBody, error: 'Invalid type' }) + continue + } + const type: EventType = eventBody.type + const body: EventProps & EventPropsByType[EventType] = eventBody + if (isSurvey(body) && body.survey_comment) { + body.survey_rating = await getSurveyCommentRating({ + comment: body.survey_comment, + language: body.context.path_language || 'en', + }) + body.survey_comment_language = await getGuessedLanguage(body.survey_comment) + } - // Validate the data matches the corresponding data schema - const validate = validators[type] - if (!validate(body)) { - // This protects so we don't bother sending the same validation - // error, per user, more than once (per time interval). - // This helps if we're bombarded with junk bot traffic. So it - // protects our Hydro instance from being overloaded with things - // that aren't helping anybody. - const hash = `${req.ip}:${(validate.errors || []) - .map((error: ErrorObject) => error.message + error.instancePath) - .join(':')}` - if (!sentValidationErrors.has(hash)) { - sentValidationErrors.set(hash, true) - // Track validation errors in Hydro so that we can know if - // there's a widespread problem in events.ts - await publish( - formatErrors(validate.errors || [], body).map((error) => ({ - schema: hydroNames.validation, - value: error, - })), - ) + if (body.context) { + // Add dotcom_user to the context if it's available + // JSON.stringify removes `undefined` values but not `null`, and we don't want to send `null` to Hydro + body.context.dotcom_user = req.cookies?.dotcom_user ? req.cookies.dotcom_user : undefined + body.context.is_staff = Boolean(req.cookies?.staffonly) + } + const validate = validators[type] + if (!validate(body)) { + validationErrors.push({ + event: body, + error: validate.errors || [], + }) + // This protects so we don't bother sending the same validation + // error, per user, more than once (per time interval). + // This helps if we're bombarded with junk bot traffic. So it + // protects our Hydro instance from being overloaded with things + // that aren't helping anybody. + const hash = `${req.ip}:${(validate.errors || []) + .map((error: ErrorObject) => error.message + error.instancePath) + .join(':')}` + if (!sentValidationErrors.has(hash)) { + sentValidationErrors.set(hash, true) + formatErrors(validate.errors || [], body).map((error) => { + validationErrors.push({ schema: hydroNames.validation, value: error }) + }) + } + continue + } + validEvents.push({ + schema: hydroNames[type], + value: omit(body, OMIT_FIELDS), + }) + } catch (eventError) { + console.error('Error validating event:', eventError) } - // We aren't helping bots spam us :) - return res.status(400).json(isProd ? {} : validate.errors) } - - if (isSurvey(body) && body.survey_comment) { - body.survey_rating = await getSurveyCommentRating({ - comment: body.survey_comment, - language: body.context.path_language || 'en', - }) - body.survey_comment_language = await getGuessedLanguage(body.survey_comment) + if (validEvents.length > 0) { + await publish(validEvents) } - // Add dotcom_user to the context if it's available - // JSON.stringify removes `undefined` values but not `null`, and we don't want to send `null` to Hydro - if (body.context) { - body.context.dotcom_user = req.cookies?.dotcom_user ? req.cookies.dotcom_user : undefined - // Add if the user is a staff, using the 'staffonly' cookie - body.context.is_staff = Boolean(req.cookies?.staffonly) + if (validationErrors.length > 0) { + await publish(validationErrors) } + const statusCode = validationErrors.length > 0 ? 400 : 200 - await publish({ - schema: hydroNames[type], - value: omit(body, OMIT_FIELDS), - }) - - return res.json({}) + return res.status(statusCode).json( + isProd + ? undefined + : { + success_count: validEvents.length, + failure_count: validationErrors.length, + details: validationErrors, + }, + ) }), ) diff --git a/src/events/tests/middleware.ts b/src/events/tests/middleware.ts index 6e95d027a637..37a75ef220ae 100644 --- a/src/events/tests/middleware.ts +++ b/src/events/tests/middleware.ts @@ -6,6 +6,10 @@ describe('POST /events', () => { vi.setConfig({ testTimeout: 60 * 1000 }) async function checkEvent(data: any) { + // if data is not an array, make it one + if (!Array.isArray(data)) { + data = [data] + } const body = JSON.stringify(data) const res = await post('/api/events', { body, @@ -47,15 +51,47 @@ describe('POST /events', () => { }, } - test('should record a page event', async () => { - const { statusCode } = await checkEvent(pageExample) + const exitExample = { + type: 'exit', + context: { + // Primitives + event_id: 'a35d7f88-3f48-4f36-ad89-5e3c8ebc3df7', + user: '703d32a8-ed0f-45f9-8d78-a913d4dc6f19', + version: '1.0.0', + created: '2020-10-02T17:12:18.620Z', + + // Content information + path: '/github/docs/issues', + hostname: 'github.com', + referrer: 'https://github.com/github/docs', + search: '?q=is%3Aissue+is%3Aopen+example+', + href: 'https://github.com/github/docs/issues?q=is%3Aissue+is%3Aopen+example+', + path_language: 'en', + + // Device information + os: 'linux', + os_version: '18.04', + browser: 'chrome', + browser_version: '85.0.4183.121', + viewport_width: 1418, + viewport_height: 501, + + // Location information + timezone: -7, + user_language: 'en-US', + }, + } + + test('should record a page and exit event', async () => { + const eventQueue = [pageExample, exitExample] + const { statusCode } = await checkEvent(eventQueue) expect(statusCode).toBe(200) }) test('should require a type', async () => { const { statusCode, body } = await checkEvent({ ...pageExample, type: undefined }) expect(statusCode).toBe(400) - expect(body).toEqual('{"message":"Invalid type"}') + expect(body).toContain('"error":"Invalid type"}') }) test('should require an event_id in uuid', async () => { diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index ae62f9530756..71de8784ed91 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -638,23 +638,27 @@ test.describe('survey', () => { // Important to set this up *before* interacting with the page // in case of possible race conditions. await page.route('**/api/events', (route, request) => { - route.fulfill({}) - expect(request.method()).toBe('POST') - const postData = JSON.parse(request.postData() || '{}') - // Skip the exit event - if (postData.type === 'exit') { - return - } - fulfilled++ - if (postData.type === 'survey' && postData.survey_vote === true) { - hasSurveyPressedEvent = true - } - if ( - postData.type === 'survey' && - postData.survey_vote === true && - postData.survey_comment === surveyComment - ) { - hasSurveySubmittedEvent = true + const postData = request.postData() + if (postData) { + const postDataArray = JSON.parse(postData) + route.fulfill({}) + expect(request.method()).toBe('POST') + fulfilled = postDataArray.length + for (const eventBody of postDataArray) { + if (eventBody.type === 'survey' && eventBody.survey_vote === true) { + hasSurveyPressedEvent = true + } + if (eventBody.type === 'survey' && eventBody.survey_vote === true) { + hasSurveyPressedEvent = true + } + if ( + eventBody.type === 'survey' && + eventBody.survey_vote === true && + eventBody.survey_comment === surveyComment + ) { + hasSurveySubmittedEvent = true + } + } } // At the time of writing you can't get the posted payload // when you use `navigator.sendBeacon(url, data)`. @@ -673,16 +677,28 @@ test.describe('survey', () => { await expect(page.getByRole('button', { name: 'Cancel' })).toBeVisible() await expect(page.getByRole('button', { name: 'Send' })).toBeVisible() - await page.locator('[for=survey-comment]').click() await page.locator('[for=survey-comment]').fill(surveyComment) await page.locator('[name=survey-email]').click() await page.locator('[name=survey-email]').fill('test@example.com') await page.getByRole('button', { name: 'Send' }).click() + // simulate sending an exit event to trigger sending all queued events + await page.evaluate(() => { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'hidden' + }, + }) + document.dispatchEvent(new Event('visibilitychange')) + return new Promise((resolve) => setTimeout(resolve, 100)) + }) + // Events: // 1. page view event when navigating to the page // 2. Survey thumbs up event // 3. Survey submit event - expect(fulfilled).toBe(1 + 1 + 1) + // 4. Exit event + expect(fulfilled).toBe(1 + 1 + 1 + 1) expect(hasSurveyPressedEvent).toBe(true) expect(hasSurveySubmittedEvent).toBe(true) await expect(page.getByTestId('survey-end')).toBeVisible() @@ -691,20 +707,22 @@ test.describe('survey', () => { test('thumbs up without filling in the form sends an API POST', async ({ page }) => { let fulfilled = 0 let hasSurveyEvent = false + // Important to set this up *before* interacting with the page // in case of possible race conditions. await page.route('**/api/events', (route, request) => { - route.fulfill({}) - expect(request.method()).toBe('POST') - const postData = JSON.parse(request.postData() || '{}') - // Skip the exit event - if (postData.type === 'exit') { - return - } - if (postData.type === 'survey' && postData.survey_vote === true) { - hasSurveyEvent = true + const postData = request.postData() + if (postData) { + const postDataArray = JSON.parse(postData) + route.fulfill({}) + expect(request.method()).toBe('POST') + fulfilled = postDataArray.length + for (const eventBody of postDataArray) { + if (eventBody.type === 'survey' && eventBody.survey_vote === true) { + hasSurveyEvent = true + } + } } - fulfilled++ // At the time of writing you can't get the posted payload // when you use `navigator.sendBeacon(url, data)`. // So we can't make assertions about the payload. @@ -718,10 +736,22 @@ test.describe('survey', () => { await page.goto('/get-started/foo/for-playwright') await page.locator('[for=survey-yes]').click() + // simulate sending an exit event to trigger sending all queued events + await page.evaluate(() => { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'hidden' + }, + }) + document.dispatchEvent(new Event('visibilitychange')) + return new Promise((resolve) => setTimeout(resolve, 100)) + }) // Events: // 1. page view event when navigating to the page // 2. the thumbs up click - expect(fulfilled).toBe(1 + 1) + // 3. the exit event + expect(fulfilled).toBe(1 + 1 + 1) expect(hasSurveyEvent).toBe(true) await expect(page.getByRole('button', { name: 'Send' })).toBeVisible()