From 00f76d610114d2aa4d3b1edc053bd66f9791f21e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Feb 2023 12:49:41 +0100 Subject: [PATCH 1/5] test(replay): Add tests for error-mode and error linking --- .../suites/replay/errors/droppedError/init.js | 18 +++ .../suites/replay/errors/droppedError/test.ts | 122 ++++++++++++++++ .../suites/replay/errors/errorMode/test.ts | 131 ++++++++++++++++++ .../replay/errors/errorsInSession/init.js | 16 +++ .../replay/errors/errorsInSession/test.ts | 77 ++++++++++ .../suites/replay/errors/init.js | 16 +++ .../suites/replay/errors/subject.js | 14 ++ .../suites/replay/errors/template.html | 11 ++ .../utils/replayEventTemplates.ts | 12 ++ 9 files changed, 417 insertions(+) create mode 100644 packages/integration-tests/suites/replay/errors/droppedError/init.js create mode 100644 packages/integration-tests/suites/replay/errors/droppedError/test.ts create mode 100644 packages/integration-tests/suites/replay/errors/errorMode/test.ts create mode 100644 packages/integration-tests/suites/replay/errors/errorsInSession/init.js create mode 100644 packages/integration-tests/suites/replay/errors/errorsInSession/test.ts create mode 100644 packages/integration-tests/suites/replay/errors/init.js create mode 100644 packages/integration-tests/suites/replay/errors/subject.js create mode 100644 packages/integration-tests/suites/replay/errors/template.html diff --git a/packages/integration-tests/suites/replay/errors/droppedError/init.js b/packages/integration-tests/suites/replay/errors/droppedError/init.js new file mode 100644 index 000000000000..bd8259208409 --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/droppedError/init.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + beforeSend(_) { + return null; + }, + integrations: [window.Replay], +}); diff --git a/packages/integration-tests/suites/replay/errors/droppedError/test.ts b/packages/integration-tests/suites/replay/errors/droppedError/test.ts new file mode 100644 index 000000000000..6c63f0bd7fb9 --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/droppedError/test.ts @@ -0,0 +1,122 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { + expectedClickBreadcrumb, + expectedConsoleBreadcrumb, + getExpectedReplayEvent, +} from '../../../../utils/replayEventTemplates'; +import { + getReplayEvent, + getReplayRecordingContent, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../../utils/replayHelpers'; + +sentryTest( + '[error-mode] should start recording if an error occurred although the error was dropped', + async ({ getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + let callsToSentry = 0; + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + const reqPromise2 = waitForReplayRequest(page, 2); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + callsToSentry++; + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await page.click('#go-background'); + expect(callsToSentry).toEqual(0); + + await page.click('#error'); + const req0 = await reqPromise0; + + await page.click('#go-background'); + const req1 = await reqPromise1; + expect(callsToSentry).toEqual(2); // 2 replay events + + await page.click('#log'); + await page.click('#go-background'); + const req2 = await reqPromise2; + + const event0 = getReplayEvent(req0); + const content0 = getReplayRecordingContent(req0); + + const event1 = getReplayEvent(req1); + const content1 = getReplayRecordingContent(req1); + + const event2 = getReplayEvent(req2); + const content2 = getReplayRecordingContent(req2); + + expect(event0).toEqual( + getExpectedReplayEvent({ + contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, + // This is by design. A dropped error shouldn't be in this list. + error_ids: [], + replay_type: 'error', + }), + ); + + // The first event should have both, full and incremental snapshots, + // as we recorded and kept all events in the buffer + expect(content0.fullSnapshots).toHaveLength(1); + expect(content0.incrementalSnapshots).toHaveLength(10); + // We want to make sure that the event that triggered the error was recorded. + expect(content0.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button#error', + }, + ]), + ); + + expect(event1).toEqual( + getExpectedReplayEvent({ + contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, + // @ts-ignore this is fine + replay_type: 'error', // although we're in session mode, we still send 'error' as replay_type + replay_start_timestamp: undefined, + segment_id: 1, + urls: [], + }), + ); + + // Also the second snapshot should have a full snapshot, as we switched from error to session + // mode which triggers another checkout + expect(content1.fullSnapshots).toHaveLength(1); + expect(content1.incrementalSnapshots).toHaveLength(0); + + // The next event should just be a normal replay event as we're now in session mode and + // we continue recording everything + expect(event2).toEqual( + getExpectedReplayEvent({ + contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, + // @ts-ignore this is fine + replay_type: 'error', + replay_start_timestamp: undefined, + segment_id: 2, + urls: [], + }), + ); + + expect(content2.breadcrumbs).toEqual( + expect.arrayContaining([ + { ...expectedClickBreadcrumb, message: 'body > button#log' }, + { ...expectedConsoleBreadcrumb, level: 'log', message: 'Some message' }, + ]), + ); + }, +); diff --git a/packages/integration-tests/suites/replay/errors/errorMode/test.ts b/packages/integration-tests/suites/replay/errors/errorMode/test.ts new file mode 100644 index 000000000000..649104e397c2 --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/errorMode/test.ts @@ -0,0 +1,131 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser } from '../../../../utils/helpers'; +import { + expectedClickBreadcrumb, + expectedConsoleBreadcrumb, + getExpectedReplayEvent, +} from '../../../../utils/replayEventTemplates'; +import { + getReplayEvent, + getReplayRecordingContent, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../../utils/replayHelpers'; + +sentryTest( + '[error-mode] should start recording and switch to session mode once an error is thrown', + async ({ getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + let callsToSentry = 0; + let errorEventId: string | undefined; + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + const reqPromise2 = waitForReplayRequest(page, 2); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + const event = envelopeRequestParser(route.request()); + // error event have no type field + if (event && !event.type && event.event_id) { + errorEventId = event.event_id; + } + callsToSentry++; + + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await page.click('#go-background'); + expect(callsToSentry).toEqual(0); + + await page.click('#error'); + const req0 = await reqPromise0; + + await page.click('#go-background'); + const req1 = await reqPromise1; + + expect(callsToSentry).toEqual(3); // 1 error, 2 replay events + + await page.click('#log'); + await page.click('#go-background'); + const req2 = await reqPromise2; + + const event0 = getReplayEvent(req0); + const content0 = getReplayRecordingContent(req0); + + const event1 = getReplayEvent(req1); + const content1 = getReplayRecordingContent(req1); + + const event2 = getReplayEvent(req2); + const content2 = getReplayRecordingContent(req2); + + expect(event0).toEqual( + getExpectedReplayEvent({ + contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, + // @ts-ignore this is fine + error_ids: [errorEventId], + replay_type: 'error', + }), + ); + + // The first event should have both, full and incremental snapshots, + // as we recorded and kept all events in the buffer + expect(content0.fullSnapshots).toHaveLength(1); + expect(content0.incrementalSnapshots).toHaveLength(10); + // We want to make sure that the event that triggered the error was recorded. + expect(content0.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button#error', + }, + ]), + ); + + expect(event1).toEqual( + getExpectedReplayEvent({ + contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, + // @ts-ignore this is fine + replay_type: 'error', // although we're in session mode, we still send 'error' as replay_type + replay_start_timestamp: undefined, + segment_id: 1, + urls: [], + }), + ); + + // Also the second snapshot should have a full snapshot, as we switched from error to session + // mode which triggers another checkout + expect(content1.fullSnapshots).toHaveLength(1); + expect(content1.incrementalSnapshots).toHaveLength(0); + + // The next event should just be a normal replay event as we're now in session mode and + // we continue recording everything + expect(event2).toEqual( + getExpectedReplayEvent({ + contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, + // @ts-ignore this is fine + replay_type: 'error', + replay_start_timestamp: undefined, + segment_id: 2, + urls: [], + }), + ); + + expect(content2.breadcrumbs).toEqual( + expect.arrayContaining([ + { ...expectedClickBreadcrumb, message: 'body > button#log' }, + { ...expectedConsoleBreadcrumb, level: 'log', message: 'Some message' }, + ]), + ); + }, +); diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/init.js b/packages/integration-tests/suites/replay/errors/errorsInSession/init.js new file mode 100644 index 000000000000..bd67decc186e --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/init.js @@ -0,0 +1,16 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + integrations: [window.Replay], +}); diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts new file mode 100644 index 000000000000..f3268635886c --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -0,0 +1,77 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser } from '../../../../utils/helpers'; +import { expectedClickBreadcrumb, getExpectedReplayEvent } from '../../../../utils/replayEventTemplates'; +import { + getReplayEvent, + getReplayRecordingContent, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../../utils/replayHelpers'; + +sentryTest( + '[session-mode] replay event should contain an error id of an error that occurred during session recording', + async ({ getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + let errorEventId: string = 'invalid_id'; + + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + const event = envelopeRequestParser(route.request()); + // error event have no type field + if (event && !event.type && event.event_id) { + errorEventId = event.event_id; + } + + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await page.click('#go-background'); + const req0 = await reqPromise0; + + await page.click('#error'); + await page.click('#go-background'); + const req1 = await reqPromise1; + + const event0 = getReplayEvent(req0); + const content0 = getReplayRecordingContent(req0); + + const event1 = getReplayEvent(req1); + const content1 = getReplayRecordingContent(req1); + + expect(event0).toEqual(getExpectedReplayEvent()); + + // The first event should have both, full and incremental snapshots, + // as we recorded and kept all events in the buffer + expect(content0.fullSnapshots).toHaveLength(1); + expect(content0.incrementalSnapshots).toHaveLength(0); + + expect(event1).toEqual( + getExpectedReplayEvent({ + replay_start_timestamp: undefined, + segment_id: 1, + // @ts-ignore this is fine + error_ids: [errorEventId], + urls: [], + }), + ); + + // Also the second snapshot should have a full snapshot, as we switched from error to session + // mode which triggers another checkout + expect(content1.fullSnapshots).toHaveLength(0); + expect(content1.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); + }, +); diff --git a/packages/integration-tests/suites/replay/errors/init.js b/packages/integration-tests/suites/replay/errors/init.js new file mode 100644 index 000000000000..d34480954ef5 --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/init.js @@ -0,0 +1,16 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + + integrations: [window.Replay], +}); diff --git a/packages/integration-tests/suites/replay/errors/subject.js b/packages/integration-tests/suites/replay/errors/subject.js new file mode 100644 index 000000000000..21d52fc179d8 --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/subject.js @@ -0,0 +1,14 @@ +document.getElementById('go-background').addEventListener('click', () => { + Object.defineProperty(document, 'hidden', { value: true, writable: true }); + const ev = document.createEvent('Event'); + ev.initEvent('visibilitychange'); + document.dispatchEvent(ev); +}); + +document.getElementById('error').addEventListener('click', () => { + throw new Error('Ooops'); +}); + +document.getElementById('log').addEventListener('click', () => { + console.log('Some message'); +}); diff --git a/packages/integration-tests/suites/replay/errors/template.html b/packages/integration-tests/suites/replay/errors/template.html new file mode 100644 index 000000000000..a034314ae2eb --- /dev/null +++ b/packages/integration-tests/suites/replay/errors/template.html @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/packages/integration-tests/utils/replayEventTemplates.ts b/packages/integration-tests/utils/replayEventTemplates.ts index 6ab8ebb8e52b..cabb27d5d6e9 100644 --- a/packages/integration-tests/utils/replayEventTemplates.ts +++ b/packages/integration-tests/utils/replayEventTemplates.ts @@ -173,3 +173,15 @@ export const expectedNavigationBreadcrumb = { to: expect.any(String), }, }; + +export const expectedConsoleBreadcrumb = { + timestamp: expect.any(Number), + type: 'default', + category: 'console', + data: { + logger: 'console', + arguments: expect.any(Array), + }, + level: expect.stringMatching(/(log|warn|error)/), + message: expect.any(String), +}; From 3a12c532d49be6954b31ac4169eb9b7b989b01f9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Feb 2023 13:46:13 +0100 Subject: [PATCH 2/5] ease up nr of incremental snapshots --- .../suites/replay/errors/droppedError/test.ts | 4 +++- .../suites/replay/errors/errorMode/test.ts | 6 ++++-- .../suites/replay/errors/errorsInSession/test.ts | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/integration-tests/suites/replay/errors/droppedError/test.ts b/packages/integration-tests/suites/replay/errors/droppedError/test.ts index 6c63f0bd7fb9..fffc9dc5b594 100644 --- a/packages/integration-tests/suites/replay/errors/droppedError/test.ts +++ b/packages/integration-tests/suites/replay/errors/droppedError/test.ts @@ -72,7 +72,9 @@ sentryTest( // The first event should have both, full and incremental snapshots, // as we recorded and kept all events in the buffer expect(content0.fullSnapshots).toHaveLength(1); - expect(content0.incrementalSnapshots).toHaveLength(10); + // We don't know how many incremental snapshots we'll have (also browser-dependent), + // but we know that we have at least 5 + expect(content0.incrementalSnapshots.length).toBeGreaterThan(5); // We want to make sure that the event that triggered the error was recorded. expect(content0.breadcrumbs).toEqual( expect.arrayContaining([ diff --git a/packages/integration-tests/suites/replay/errors/errorMode/test.ts b/packages/integration-tests/suites/replay/errors/errorMode/test.ts index 649104e397c2..839a5e1ffa46 100644 --- a/packages/integration-tests/suites/replay/errors/errorMode/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorMode/test.ts @@ -29,7 +29,7 @@ sentryTest( await page.route('https://dsn.ingest.sentry.io/**/*', route => { const event = envelopeRequestParser(route.request()); - // error event have no type field + // error events have no type field if (event && !event.type && event.event_id) { errorEventId = event.event_id; } @@ -81,7 +81,9 @@ sentryTest( // The first event should have both, full and incremental snapshots, // as we recorded and kept all events in the buffer expect(content0.fullSnapshots).toHaveLength(1); - expect(content0.incrementalSnapshots).toHaveLength(10); + // We don't know how many incremental snapshots we'll have (also browser-dependent), + // but we know that we have at least 5 + expect(content0.incrementalSnapshots.length).toBeGreaterThan(5); // We want to make sure that the event that triggered the error was recorded. expect(content0.breadcrumbs).toEqual( expect.arrayContaining([ diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index f3268635886c..c404db23378b 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -24,7 +24,7 @@ sentryTest( await page.route('https://dsn.ingest.sentry.io/**/*', route => { const event = envelopeRequestParser(route.request()); - // error event have no type field + // error events have no type field if (event && !event.type && event.event_id) { errorEventId = event.event_id; } From d585acf471098f5c4235c871da76dd99e7555989 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Feb 2023 14:48:36 +0100 Subject: [PATCH 3/5] streamline sessionmode test and add another one --- .../replay/errors/errorsInSession/init.js | 8 ++- .../replay/errors/errorsInSession/test.ts | 51 +++++++++++++++---- .../suites/replay/errors/subject.js | 4 ++ .../suites/replay/errors/template.html | 1 + 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/init.js b/packages/integration-tests/suites/replay/errors/errorsInSession/init.js index bd67decc186e..019777c27156 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/init.js +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/init.js @@ -11,6 +11,12 @@ Sentry.init({ sampleRate: 1, replaysSessionSampleRate: 1.0, replaysOnErrorSampleRate: 0.0, - + beforeSend(event, hint) { + if (hint.originalException.message.includes('[drop]')) { + return null; + } + return event; + }, integrations: [window.Replay], + debug: true, }); diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index c404db23378b..0c66964b6892 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -47,18 +47,12 @@ sentryTest( const req1 = await reqPromise1; const event0 = getReplayEvent(req0); - const content0 = getReplayRecordingContent(req0); const event1 = getReplayEvent(req1); const content1 = getReplayRecordingContent(req1); expect(event0).toEqual(getExpectedReplayEvent()); - // The first event should have both, full and incremental snapshots, - // as we recorded and kept all events in the buffer - expect(content0.fullSnapshots).toHaveLength(1); - expect(content0.incrementalSnapshots).toHaveLength(0); - expect(event1).toEqual( getExpectedReplayEvent({ replay_start_timestamp: undefined, @@ -69,9 +63,48 @@ sentryTest( }), ); - // Also the second snapshot should have a full snapshot, as we switched from error to session - // mode which triggers another checkout - expect(content1.fullSnapshots).toHaveLength(0); + expect(content1.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); + }, +); + +sentryTest( + '[session-mode] replay event should not contain an error id of a dropped error while recording', + async ({ getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + const reqPromise1 = waitForReplayRequest(page, 1); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await page.click('#go-background'); + + await page.click('#drop'); + await page.click('#go-background'); + const req1 = await reqPromise1; + + const event1 = getReplayEvent(req1); + const content1 = getReplayRecordingContent(req1); + + expect(event1).toEqual( + getExpectedReplayEvent({ + replay_start_timestamp: undefined, + segment_id: 1, + error_ids: [], // <-- no error id + urls: [], + }), + ); + expect(content1.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); }, ); diff --git a/packages/integration-tests/suites/replay/errors/subject.js b/packages/integration-tests/suites/replay/errors/subject.js index 21d52fc179d8..de82a997f957 100644 --- a/packages/integration-tests/suites/replay/errors/subject.js +++ b/packages/integration-tests/suites/replay/errors/subject.js @@ -9,6 +9,10 @@ document.getElementById('error').addEventListener('click', () => { throw new Error('Ooops'); }); +document.getElementById('drop').addEventListener('click', () => { + throw new Error('[drop] Ooops'); +}); + document.getElementById('log').addEventListener('click', () => { console.log('Some message'); }); diff --git a/packages/integration-tests/suites/replay/errors/template.html b/packages/integration-tests/suites/replay/errors/template.html index a034314ae2eb..99dcde1a9a88 100644 --- a/packages/integration-tests/suites/replay/errors/template.html +++ b/packages/integration-tests/suites/replay/errors/template.html @@ -6,6 +6,7 @@ + From f0b35762e3133aa882510ffb7aaba2100829b4e1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Feb 2023 15:04:28 +0100 Subject: [PATCH 4/5] streamline error-mode->droppedError test --- .../suites/replay/errors/droppedError/test.ts | 91 ++++--------------- 1 file changed, 16 insertions(+), 75 deletions(-) diff --git a/packages/integration-tests/suites/replay/errors/droppedError/test.ts b/packages/integration-tests/suites/replay/errors/droppedError/test.ts index fffc9dc5b594..c509dda8206e 100644 --- a/packages/integration-tests/suites/replay/errors/droppedError/test.ts +++ b/packages/integration-tests/suites/replay/errors/droppedError/test.ts @@ -1,18 +1,22 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { - expectedClickBreadcrumb, - expectedConsoleBreadcrumb, - getExpectedReplayEvent, -} from '../../../../utils/replayEventTemplates'; -import { - getReplayEvent, - getReplayRecordingContent, - shouldSkipReplayTest, - waitForReplayRequest, -} from '../../../../utils/replayHelpers'; - +import { getExpectedReplayEvent } from '../../../../utils/replayEventTemplates'; +import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +/* + * This scenario currently shows somewhat unexpected behavior from the PoV of a user: + * The error is dropped, but the recording is started and continued anyway. + * If folks only sample error replays, this will lead to a lot of confusion as the resulting replay + * won't contain the error that started it (possibly none or only additional errors that occurred later on). + * + * This is because in error-mode, we start recording as soon as replay's eventProcessor is called with an error. + * If later event processors or beforeSend drop the error, the recording is already started. + * + * We'll need a proper SDK lifecycle hook (WIP) to fix this properly. + * TODO: Once we have lifecycle hooks, we should revisit this test and make sure it behaves as expected. + * This means that the recording should not be started or stopped if the error that triggered it is not sent. + */ sentryTest( '[error-mode] should start recording if an error occurred although the error was dropped', async ({ getLocalTestPath, page }) => { @@ -22,8 +26,6 @@ sentryTest( let callsToSentry = 0; const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise1 = waitForReplayRequest(page, 1); - const reqPromise2 = waitForReplayRequest(page, 2); await page.route('https://dsn.ingest.sentry.io/**/*', route => { callsToSentry++; @@ -44,21 +46,12 @@ sentryTest( const req0 = await reqPromise0; await page.click('#go-background'); - const req1 = await reqPromise1; expect(callsToSentry).toEqual(2); // 2 replay events await page.click('#log'); await page.click('#go-background'); - const req2 = await reqPromise2; const event0 = getReplayEvent(req0); - const content0 = getReplayRecordingContent(req0); - - const event1 = getReplayEvent(req1); - const content1 = getReplayRecordingContent(req1); - - const event2 = getReplayEvent(req2); - const content2 = getReplayRecordingContent(req2); expect(event0).toEqual( getExpectedReplayEvent({ @@ -68,57 +61,5 @@ sentryTest( replay_type: 'error', }), ); - - // The first event should have both, full and incremental snapshots, - // as we recorded and kept all events in the buffer - expect(content0.fullSnapshots).toHaveLength(1); - // We don't know how many incremental snapshots we'll have (also browser-dependent), - // but we know that we have at least 5 - expect(content0.incrementalSnapshots.length).toBeGreaterThan(5); - // We want to make sure that the event that triggered the error was recorded. - expect(content0.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > button#error', - }, - ]), - ); - - expect(event1).toEqual( - getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, - // @ts-ignore this is fine - replay_type: 'error', // although we're in session mode, we still send 'error' as replay_type - replay_start_timestamp: undefined, - segment_id: 1, - urls: [], - }), - ); - - // Also the second snapshot should have a full snapshot, as we switched from error to session - // mode which triggers another checkout - expect(content1.fullSnapshots).toHaveLength(1); - expect(content1.incrementalSnapshots).toHaveLength(0); - - // The next event should just be a normal replay event as we're now in session mode and - // we continue recording everything - expect(event2).toEqual( - getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, - // @ts-ignore this is fine - replay_type: 'error', - replay_start_timestamp: undefined, - segment_id: 2, - urls: [], - }), - ); - - expect(content2.breadcrumbs).toEqual( - expect.arrayContaining([ - { ...expectedClickBreadcrumb, message: 'body > button#log' }, - { ...expectedConsoleBreadcrumb, level: 'log', message: 'Some message' }, - ]), - ); }, ); From 70f85c78c430d5c84bd5c6207e1ffd364d613747 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Feb 2023 15:32:24 +0100 Subject: [PATCH 5/5] make session-mode button click breadcrumbs more specific --- .../suites/replay/errors/errorsInSession/test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index 0c66964b6892..217a19b5198a 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -63,7 +63,9 @@ sentryTest( }), ); - expect(content1.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([{ ...expectedClickBreadcrumb, message: 'body > button#error' }]), + ); }, ); @@ -105,6 +107,9 @@ sentryTest( }), ); - expect(content1.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); + // The button click that triggered the error should still be recorded + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([{ ...expectedClickBreadcrumb, message: 'body > button#drop' }]), + ); }, );