diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cd358158b2ee..d903d4913cd4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -795,7 +795,8 @@ jobs: # - The build job was successful, not skipped # - AND if the profiling node bindings were either successful or skipped if: | - always() && needs.job_build.result == 'success' && + always() && + needs.job_build.result == 'success' && (needs.job_compile_bindings_profiling_node.result == 'success' || needs.job_compile_bindings_profiling_node.result == 'skipped') needs: [job_get_metadata, job_build, job_compile_bindings_profiling_node] runs-on: ubuntu-20.04-large-js @@ -981,13 +982,16 @@ jobs: directory: dev-packages/e2e-tests token: ${{ secrets.CODECOV_TOKEN }} + # - We skip optional tests on release branches job_optional_e2e_tests: name: E2E ${{ matrix.label || matrix.test-application }} Test # We only run E2E tests for non-fork PRs because the E2E tests require secrets to work and they can't be accessed from forks # We need to add the `always()` check here because the previous step has this as well :( # See: https://github.com/actions/runner/issues/2205 if: - always() && needs.job_e2e_prepare.result == 'success' && + always() && + needs.job_get_metadata.outputs.is_release != 'true' && + needs.job_e2e_prepare.result == 'success' && needs.job_e2e_prepare.outputs.matrix-optional != '{"include":[]}' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' diff --git a/CHANGELOG.md b/CHANGELOG.md index ab6f9fb23635..1c0db74fffcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,10 +81,12 @@ ### Other Changes - feat(browser): Send additional LCP timing info ([#14372](https://github.com/getsentry/sentry-javascript/pull/14372)) +- feat(replay): Clear event buffer when full and in buffer mode ([#14078](https://github.com/getsentry/sentry-javascript/pull/14078)) - feat(core): Ensure `normalizedRequest` on `sdkProcessingMetadata` is merged ([#14315](https://github.com/getsentry/sentry-javascript/pull/14315)) - feat(core): Hoist everything from `@sentry/utils` into `@sentry/core` ([#14382](https://github.com/getsentry/sentry-javascript/pull/14382)) - fix(core): Do not throw when trying to fill readonly properties ([#14402](https://github.com/getsentry/sentry-javascript/pull/14402)) - fix(feedback): Fix `__self` and `__source` attributes on feedback nodes ([#14356](https://github.com/getsentry/sentry-javascript/pull/14356)) +- fix(feedback): Fix non-wrapping form title ([#14355](https://github.com/getsentry/sentry-javascript/pull/14355)) - fix(nextjs): Update check for not found navigation error ([#14378](https://github.com/getsentry/sentry-javascript/pull/14378)) ## 8.39.0 diff --git a/packages/feedback/src/modal/components/Dialog.css.ts b/packages/feedback/src/modal/components/Dialog.css.ts index fc0e00580a5d..da12e3d70990 100644 --- a/packages/feedback/src/modal/components/Dialog.css.ts +++ b/packages/feedback/src/modal/components/Dialog.css.ts @@ -60,7 +60,7 @@ const DIALOG = ` gap: 16px; padding: var(--dialog-padding, 24px); max-width: 100%; - width: 100%; + width: var(--form-width, 272px); max-height: 100%; overflow: auto; @@ -71,17 +71,26 @@ const DIALOG = ` transform: translate(0, 0) scale(1); transition: transform 0.2s ease-in-out; } + +@media (max-width: 600px) { + .dialog__content { + width: var(--form-width, 100%); + } +} + `; const DIALOG_HEADER = ` .dialog__header { display: flex; - align-items: center; + gap: 4px; justify-content: space-between; font-weight: var(--dialog-header-weight, 600); margin: 0; } - +.dialog__title { + align-self: center; +} .brand-link { display: inline-flex; } @@ -101,18 +110,12 @@ const FORM = ` .form__right { flex: 0 0 auto; - width: var(--form-width, 272px); display: flex; overflow: auto; flex-direction: column; justify-content: space-between; gap: 20px; -} - -@media (max-width: 600px) { - .form__right { - width: var(--form-width, 100%); - } + width: 100%; } .form__top { diff --git a/packages/feedback/src/modal/components/DialogHeader.tsx b/packages/feedback/src/modal/components/DialogHeader.tsx index 44d29af629f6..217ce6676ee0 100644 --- a/packages/feedback/src/modal/components/DialogHeader.tsx +++ b/packages/feedback/src/modal/components/DialogHeader.tsx @@ -14,7 +14,7 @@ export function DialogHeader({ options }: Props): VNode { return (

- {options.formTitle} + {options.formTitle} {options.showBranding ? ( { - const { events, hasCheckout } = this._fallback; + const { events, hasCheckout, waitForCheckout } = this._fallback; const addEventPromises: Promise[] = []; for (const event of events) { @@ -107,6 +118,7 @@ export class EventBufferProxy implements EventBuffer { } this._compression.hasCheckout = hasCheckout; + this._compression.waitForCheckout = waitForCheckout; // We switch over to the new buffer immediately - any further events will be added // after the previously buffered ones diff --git a/packages/replay-internal/src/types/replay.ts b/packages/replay-internal/src/types/replay.ts index 6892c05ee179..4f78a438ecc3 100644 --- a/packages/replay-internal/src/types/replay.ts +++ b/packages/replay-internal/src/types/replay.ts @@ -400,6 +400,12 @@ export interface EventBuffer { */ hasCheckout: boolean; + /** + * If the event buffer needs to wait for a checkout event before it + * starts buffering events. + */ + waitForCheckout: boolean; + /** * Destroy the event buffer. */ diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 3a245242e608..5aa420bfff15 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,17 +54,22 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - if (!replay.eventBuffer) { + const { eventBuffer } = replay; + + if (!eventBuffer || (eventBuffer.waitForCheckout && !isCheckout)) { return null; } + const isBufferMode = replay.recordingMode === 'buffer'; + try { - if (isCheckout && replay.recordingMode === 'buffer') { - replay.eventBuffer.clear(); + if (isCheckout && isBufferMode) { + eventBuffer.clear(); } if (isCheckout) { - replay.eventBuffer.hasCheckout = true; + eventBuffer.hasCheckout = true; + eventBuffer.waitForCheckout = false; } const replayOptions = replay.getOptions(); @@ -75,9 +80,19 @@ async function _addEvent( return; } - return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); + return await eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { - const reason = error && error instanceof EventBufferSizeExceededError ? 'addEventSizeExceeded' : 'addEvent'; + const isExceeded = error && error instanceof EventBufferSizeExceededError; + const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; + + if (isExceeded && isBufferMode) { + // Clear buffer and wait for next checkout + eventBuffer.clear(); + eventBuffer.waitForCheckout = true; + + return null; + } + replay.handleException(error); await replay.stop({ reason }); diff --git a/packages/replay-internal/test/integration/eventBuffer.test.ts b/packages/replay-internal/test/integration/eventBuffer.test.ts new file mode 100644 index 000000000000..bc796516acf8 --- /dev/null +++ b/packages/replay-internal/test/integration/eventBuffer.test.ts @@ -0,0 +1,75 @@ +/** + * @vitest-environment jsdom + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { WINDOW } from '../../src/constants'; +import type { Replay } from '../../src/integration'; +import type { ReplayContainer } from '../../src/replay'; +import { addEvent } from '../../src/util/addEvent'; + +// mock functions need to be imported first +import { BASE_TIMESTAMP, mockSdk } from '../index'; +import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent'; +import { useFakeTimers } from '../utils/use-fake-timers'; + +useFakeTimers(); + +describe('Integration | eventBuffer | Event Buffer Max Size', () => { + let replay: ReplayContainer; + let integration: Replay; + const prevLocation = WINDOW.location; + + beforeEach(async () => { + vi.setSystemTime(new Date(BASE_TIMESTAMP)); + + ({ replay, integration } = await mockSdk()); + + await vi.runAllTimersAsync(); + vi.clearAllMocks(); + }); + + afterEach(async () => { + vi.setSystemTime(new Date(BASE_TIMESTAMP)); + integration && (await integration.stop()); + Object.defineProperty(WINDOW, 'location', { + value: prevLocation, + writable: true, + }); + vi.clearAllMocks(); + }); + + it('does not add replay breadcrumb when stopped due to event buffer limit', async () => { + const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); + + vi.mock('../../src/constants', async requireActual => ({ + ...(await requireActual()), + REPLAY_MAX_EVENT_BUFFER_SIZE: 500, + })); + + await integration.stop(); + integration.startBuffering(); + + await addEvent(replay, TEST_EVENT); + + expect(replay.eventBuffer?.hasEvents).toBe(true); + expect(replay.eventBuffer?.['hasCheckout']).toBe(true); + + // This should should go over max buffer size + await addEvent(replay, TEST_EVENT); + // buffer should be cleared and wait for next checkout + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(replay.eventBuffer?.['hasCheckout']).toBe(false); + + await addEvent(replay, TEST_EVENT); + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(replay.eventBuffer?.['hasCheckout']).toBe(false); + + await addEvent(replay, getTestEventCheckout({ timestamp: Date.now() }), true); + expect(replay.eventBuffer?.hasEvents).toBe(true); + expect(replay.eventBuffer?.['hasCheckout']).toBe(true); + + vi.resetAllMocks(); + }); +}); diff --git a/packages/replay-internal/test/integration/stop.test.ts b/packages/replay-internal/test/integration/stop.test.ts index 6ebca89891b0..5c3aa49b77d8 100644 --- a/packages/replay-internal/test/integration/stop.test.ts +++ b/packages/replay-internal/test/integration/stop.test.ts @@ -3,14 +3,13 @@ */ import type { MockInstance, MockedFunction } from 'vitest'; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import { WINDOW } from '../../src/constants'; import type { Replay } from '../../src/integration'; import type { ReplayContainer } from '../../src/replay'; -import { clearSession } from '../../src/session/clearSession'; import { addEvent } from '../../src/util/addEvent'; import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; // mock functions need to be imported first @@ -32,7 +31,7 @@ describe('Integration | stop', () => { let mockAddDomInstrumentationHandler: MockInstance; let mockRunFlush: MockRunFlush; - beforeAll(async () => { + beforeEach(async () => { vi.setSystemTime(new Date(BASE_TIMESTAMP)); mockAddDomInstrumentationHandler = vi.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler'); @@ -41,33 +40,18 @@ describe('Integration | stop', () => { // @ts-expect-error private API mockRunFlush = vi.spyOn(replay, '_runFlush'); - vi.runAllTimers(); - }); - - beforeEach(() => { - vi.setSystemTime(new Date(BASE_TIMESTAMP)); - replay.eventBuffer?.destroy(); + await vi.runAllTimersAsync(); vi.clearAllMocks(); }); afterEach(async () => { - vi.runAllTimers(); - await new Promise(process.nextTick); vi.setSystemTime(new Date(BASE_TIMESTAMP)); - sessionStorage.clear(); - clearSession(replay); - replay['_initializeSessionForSampling'](); - replay.setInitialState(); - mockRecord.takeFullSnapshot.mockClear(); - mockAddDomInstrumentationHandler.mockClear(); + integration && (await integration.stop()); Object.defineProperty(WINDOW, 'location', { value: prevLocation, writable: true, }); - }); - - afterAll(() => { - integration && integration.stop(); + vi.clearAllMocks(); }); it('does not upload replay if it was stopped and can resume replays afterwards', async () => { @@ -106,7 +90,7 @@ describe('Integration | stop', () => { vi.advanceTimersByTime(ELAPSED); - const timestamp = +new Date(BASE_TIMESTAMP + ELAPSED + ELAPSED) / 1000; + const timestamp = +new Date(BASE_TIMESTAMP + ELAPSED + ELAPSED + ELAPSED) / 1000; const hiddenBreadcrumb = { type: 5, @@ -123,7 +107,7 @@ describe('Integration | stop', () => { addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); - vi.runAllTimers(); + await vi.runAllTimersAsync(); await new Promise(process.nextTick); expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 0 }, @@ -131,7 +115,7 @@ describe('Integration | stop', () => { // This event happens when we call `replay.start` { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + ELAPSED, + timestamp: BASE_TIMESTAMP + ELAPSED + ELAPSED, type: 2, }, optionsEvent, @@ -142,12 +126,14 @@ describe('Integration | stop', () => { // Session's last activity is last updated when we call `setup()` and *NOT* // when tab is blurred - expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED); + expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED + ELAPSED); }); it('does not buffer new events after being stopped', async function () { + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(mockRunFlush).toHaveBeenCalledTimes(0); const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); - addEvent(replay, TEST_EVENT); + addEvent(replay, TEST_EVENT, true); expect(replay.eventBuffer?.hasEvents).toBe(true); expect(mockRunFlush).toHaveBeenCalledTimes(0);