Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
ref(replay): Extract addMemoryEntry out
  • Loading branch information
mydea committed Dec 9, 2022
commit e02624127e69c8fb0da98b2f33a45b66309f5c58
22 changes: 3 additions & 19 deletions packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { handleGlobalEventListener } from './coreHandlers/handleGlobalEvent';
import { handleHistorySpanListener } from './coreHandlers/handleHistory';
import { handleXhrSpanListener } from './coreHandlers/handleXhr';
import { setupPerformanceObserver } from './coreHandlers/performanceObserver';
import { createMemoryEntry, createPerformanceEntries } from './createPerformanceEntry';
import { createPerformanceEntries } from './createPerformanceEntry';
import { createEventBuffer, EventBuffer } from './eventBuffer';
import { deleteSession } from './session/deleteSession';
import { getSession } from './session/getSession';
Expand All @@ -36,6 +36,7 @@ import type {
SendReplay,
} from './types';
import { addEvent } from './util/addEvent';
import { addMemoryEntry } from './util/addMemoryEntry';
import { captureInternalException } from './util/captureInternalException';
import { createBreadcrumb } from './util/createBreadcrumb';
import { createPayload } from './util/createPayload';
Expand Down Expand Up @@ -701,23 +702,6 @@ export class ReplayContainer {
return createPerformanceSpans(this, createPerformanceEntries(entries));
}

/**
* Create a "span" for the total amount of memory being used by JS objects
* (including v8 internal objects).
*/
addMemoryEntry(): Promise<void[]> | undefined {
// window.performance.memory is a non-standard API and doesn't work on all browsers
// so we check before creating the event.
if (!('memory' in WINDOW.performance)) {
return;
}

return createPerformanceSpans(this, [
// @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above)
createMemoryEntry(WINDOW.performance.memory),
]);
}

/**
* Checks if recording should be stopped due to user inactivity. Otherwise
* check if session is expired and create a new session if so. Triggers a new
Expand Down Expand Up @@ -819,7 +803,7 @@ export class ReplayContainer {
}

// Only attach memory event if eventBuffer is not empty
await this.addMemoryEntry();
await addMemoryEntry(this);

try {
// Note this empties the event buffer regardless of outcome of sending replay
Expand Down
20 changes: 20 additions & 0 deletions packages/replay/src/util/addMemoryEntry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { WINDOW } from '../constants';
import { ReplayContainer } from '../replay';
import { createPerformanceSpans } from './createPerformanceSpans';

/**
* Create a "span" for the total amount of memory being used by JS objects
* (including v8 internal objects).
*/
export function addMemoryEntry(replay: ReplayContainer): Promise<void[]> | undefined {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: can we change the func signature here to just be void? Then we don't need the return statements and we can also just void createPerformanceSpans

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good finding, there is actually nothing async about this 😅 so should be good to clean up. done!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addEvent should be async

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addEvent was also sync before, and we are not really waiting for it anywhere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 , it may have been a bug -- eventbuffer's addevent is async, looks like replay.addEvent was not returning the promise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think you're right - but I'll make a separate PR to fix that 👍

// window.performance.memory is a non-standard API and doesn't work on all browsers
// so we check before creating the event.
if (!('memory' in WINDOW.performance)) {
return;
}

return createPerformanceSpans(replay, [
// @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above)
createMemoryEntry(WINDOW.performance.memory),
]);
}
6 changes: 3 additions & 3 deletions packages/replay/test/unit/flush.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as SentryUtils from '@sentry/utils';

import { SESSION_IDLE_DURATION, WINDOW } from '../../src/constants';
import * as AddMemoryEntry from '../../src/util/addMemoryEntry';
import { createPerformanceSpans } from '../../src/util/createPerformanceSpans';
import { createPerformanceEntries } from './../../src/createPerformanceEntry';
import { ReplayContainer } from './../../src/replay';
Expand All @@ -16,7 +17,7 @@ async function advanceTimers(time: number) {

type MockSendReplay = jest.MockedFunction<typeof ReplayContainer.prototype.sendReplay>;
type MockAddPerformanceEntries = jest.MockedFunction<typeof ReplayContainer.prototype.addPerformanceEntries>;
type MockAddMemoryEntry = jest.MockedFunction<typeof ReplayContainer.prototype.addMemoryEntry>;
type MockAddMemoryEntry = jest.SpyInstance;
type MockEventBufferFinish = jest.MockedFunction<Exclude<typeof ReplayContainer.prototype.eventBuffer, null>['finish']>;
type MockFlush = jest.MockedFunction<typeof ReplayContainer.prototype.flush>;
type MockRunFlush = jest.MockedFunction<typeof ReplayContainer.prototype.runFlush>;
Expand Down Expand Up @@ -63,8 +64,7 @@ beforeAll(async () => {
return [];
});

jest.spyOn(replay, 'addMemoryEntry');
mockAddMemoryEntry = replay.addMemoryEntry as MockAddMemoryEntry;
mockAddMemoryEntry = jest.spyOn(AddMemoryEntry, 'addMemoryEntry');
});

beforeEach(() => {
Expand Down