refactor: added helper function performSessionEnd #1113

jaissica12 · 2025-11-05T15:30:26Z

Background

Session timeout calculations were duplicated in initialize() and endSession() methods
Both methods checked for session timeout but used different approaches:
- initialize() used Date object comparison
- endSession() used millisecond arithmetic with inverted logic
Session end operations (logging event, clearing data, nullifying session) were scattered inline within endSession()

What Has Changed

Added performSessionEnd helper - Extracted session end operations into a dedicated function
Added hasSessionTimedOut helper - Consolidates duplicate session timeout calculation logic

Screenshots/Video

{Include any screenshots or video demonstrating the new feature or fix, if applicable}

Checklist

I have performed a self-review of my own code.
I have made corresponding changes to the documentation.
I have added tests that prove my fix is effective or that my feature works.
I have tested this locally.

Additional Notes

The original code used different comparison operators:
initialize() used > - session expired after 30:00.000
endSession() used >= - session expired at 30:00.000

As of now I have standardized on >=, let me know if you think otherwise

Reference Issue (For employees only. Ignore if you are an outside contributor)

Closes https://go/j/SDKE-376

…376-refactor-endSession-in-sessionManager

alexs-mparticle · 2025-12-10T16:22:03Z

src/sessionManager.ts

+    function hasSessionTimedOut(lastEventTimestamp: number): boolean {
+        const sessionTimeoutInMilliseconds: number =
+            mpInstance._Store.SDKConfig.sessionTimeout * 60000;
+        const timeSinceLastEvent: number =
+            new Date().getTime() - lastEventTimestamp;
+
+        return timeSinceLastEvent >= sessionTimeoutInMilliseconds;
+    }


Let's avoid pulling in the entire store when we only need a single value.

Suggested change

function hasSessionTimedOut(lastEventTimestamp: number): boolean {

const sessionTimeoutInMilliseconds: number =

mpInstance._Store.SDKConfig.sessionTimeout * 60000;

const timeSinceLastEvent: number =

new Date().getTime() - lastEventTimestamp;

return timeSinceLastEvent >= sessionTimeoutInMilliseconds;

}

function hasSessionTimedOut(lastEventTimestamp: number, sessionTimeout: number): boolean {

const sessionTimeoutInMilliseconds: number = sessionTimeout * 60000;

const timeSinceLastEvent: number =

new Date().getTime() - lastEventTimestamp;

return timeSinceLastEvent >= sessionTimeoutInMilliseconds;

}

alexs-mparticle · 2025-12-10T16:23:32Z

src/sessionManager.ts

+        const sessionTimeoutInMilliseconds: number =
+            mpInstance._Store.SDKConfig.sessionTimeout * 60000;
+        const timeSinceLastEvent: number =
+            new Date().getTime() - lastEventTimestamp;


I think I agree with SonarCloud. let's use Date.now() instead of new Date().getTime().

alexs-mparticle · 2025-12-10T16:25:32Z

src/sessionManager.ts

-                        sessionTimeoutInMilliseconds
-                )
-            ) {
+            if (hasSessionTimedOut(mpInstance._Store.dateLastEventSent.getTime())) {


If you follow my suggestion below, this can be destructured and cleaned up.

Suggested change

if (hasSessionTimedOut(mpInstance._Store.dateLastEventSent.getTime())) {

const { dateLastEventSent, SDKConfig } = mpInstance._Store;

const { sessionTimeout } = SDKConfig;

if (hasSessionTimedOut(dateLastEventSent.getTime(), sessionTimeout)) {

Thanks for suggesting this, cleaned up initialize function

alexs-mparticle · 2025-12-10T16:26:59Z

src/sessionManager.ts

+        mpInstance._Events.logEvent({
+            messageType: Types.MessageType.SessionEnd,
+        });
+        mpInstance._Store.sessionStartDate = null;


Can we just add this line to nullifySession()?

Added this.sessionStartDate = null; to Store. nullifySession()

Copilot

Pull request overview

This PR refactors session management logic by extracting two helper functions to eliminate code duplication and improve maintainability. The refactoring consolidates timeout calculations that previously used different comparison approaches and extracts session end operations into a reusable function.

Key Changes:

Extracted hasSessionTimedOut() helper to unify session timeout logic previously duplicated between initialize() and endSession() with different comparison operators
Extracted performSessionEnd() helper to consolidate session end operations (logging, clearing data, resetting timer)
Standardized on >= comparison operator for timeout checks (expires at exactly 30 minutes)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
src/sessionManager.ts	Refactored session timeout and end logic into two helper functions; simplified `initialize()` and `endSession()` by delegating to the new helpers
src/store.ts	Updated `nullifySession()` to include clearing `sessionStartDate`, consolidating cleanup logic
test/src/tests-session-manager.ts	Added comprehensive test coverage for both new helper functions with 11 new test cases covering timeout scenarios, session end operations, and edge cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T14:55:41Z

test/src/tests-session-manager.ts

+                const now = new Date();
+                const exactlyThirtyMinutesAgo = new Date();
+                exactlyThirtyMinutesAgo.setMinutes(now.getMinutes() - 30);
+
+                mParticle.init(apiKey, window.mParticle.config);
+                const mpInstance = mParticle.getInstance();
+
+                sinon.stub(mpInstance._Persistence, 'getPersistence').returns({
+                    gs: {
+                        les: exactlyThirtyMinutesAgo.getTime(),
+                        sid: 'TEST-ID',
+                    },
+                });
+
+                mpInstance._SessionManager.endSession();
+
+                // At exactly 30 minutes, session should be expired
+                expect(mpInstance._Store.sessionId).to.equal(null);


This test has a potential timing issue. The test creates exactlyThirtyMinutesAgo using the current time, but by the time hasSessionTimedOut() is called internally (which uses Date.now()), a few milliseconds may have passed. This could make the elapsed time slightly more than 30 minutes, causing the test to pass for the wrong reason. Consider using fake timers (sinon.useFakeTimers) to control time precisely, or add a small buffer to ensure the test is deterministic.

Suggested change

const now = new Date();

const exactlyThirtyMinutesAgo = new Date();

exactlyThirtyMinutesAgo.setMinutes(now.getMinutes() - 30);

mParticle.init(apiKey, window.mParticle.config);

const mpInstance = mParticle.getInstance();

sinon.stub(mpInstance._Persistence, 'getPersistence').returns({

gs: {

les: exactlyThirtyMinutesAgo.getTime(),

sid: 'TEST-ID',

},

});

mpInstance._SessionManager.endSession();

// At exactly 30 minutes, session should be expired

expect(mpInstance._Store.sessionId).to.equal(null);

const now = Date.now();

const clock = sinon.useFakeTimers(now);

const thirtyMinutesInMs = 30 * 60 * 1000;

const exactlyThirtyMinutesAgo = new Date(now - thirtyMinutesInMs);

mParticle.init(apiKey, window.mParticle.config);

const mpInstance = mParticle.getInstance();

const getPersistenceStub = sinon.stub(mpInstance._Persistence, 'getPersistence').returns({

gs: {

les: exactlyThirtyMinutesAgo.getTime(),

sid: 'TEST-ID',

},

});

try {

mpInstance._SessionManager.endSession();

// At exactly 30 minutes, session should be expired

expect(mpInstance._Store.sessionId).to.equal(null);

} finally {

getPersistenceStub.restore();

clock.restore();

}

@jaissica12 let's test this in case the robot is telling the truth

Copilot · 2025-12-19T14:55:41Z

src/sessionManager.ts

            }
        }
-
        mpInstance._timeOnSiteTimer?.resetTimer();


The timer reset at this line is redundant when the session has timed out, because performSessionEnd() already resets the timer at line 238. This results in the timer being reset twice when a session times out via the endSession() method. Consider removing this line or moving the timer reset logic into the else block at line 170 to avoid the duplication.

@jaissica12 Can you try making this code change and see if it works? I think it may clean up the rafactor more.

Copilot · 2025-12-19T14:55:41Z

test/src/tests-session-manager.ts

+                sinon.stub(mpInstance._Persistence, 'getPersistence').returns({
+                    gs: {
+                        les: thirtyOneMinutesAgo.getTime(),
+                        sid: newSessionId,
+                    },
+                });


The sinon.stub is not cleaned up after the test. This stub will persist and could affect subsequent tests in this test suite. Use sinon.stub().returns() and restore it in a cleanup hook, or use sinon.restore() at the end of the test, or better yet, wrap this test in a context with its own beforeEach/afterEach to handle stub lifecycle.

Copilot · 2025-12-19T14:55:42Z

test/src/tests-session-manager.ts

+                sinon.stub(mpInstance._Persistence, 'getPersistence').returns({
+                    gs: {
+                        les: exactlyThirtyMinutesAgo.getTime(),
+                        sid: 'TEST-ID',
+                    },
+                });


The sinon.stub is not cleaned up after the test. This stub will persist and could affect subsequent tests in this test suite. Use sinon.stub().returns() and restore it in a cleanup hook, or use sinon.restore() at the end of the test, or better yet, wrap this test in a context with its own beforeEach/afterEach to handle stub lifecycle.

alexs-mparticle · 2025-12-19T15:00:10Z

src/sessionManager.ts

            }
        }
-
        mpInstance._timeOnSiteTimer?.resetTimer();


@jaissica12 Can you try making this code change and see if it works? I think it may clean up the rafactor more.

alexs-mparticle · 2025-12-19T15:04:31Z

test/src/tests-session-manager.ts

+                const now = new Date();
+                const exactlyThirtyMinutesAgo = new Date();
+                exactlyThirtyMinutesAgo.setMinutes(now.getMinutes() - 30);
+
+                mParticle.init(apiKey, window.mParticle.config);
+                const mpInstance = mParticle.getInstance();
+
+                sinon.stub(mpInstance._Persistence, 'getPersistence').returns({
+                    gs: {
+                        les: exactlyThirtyMinutesAgo.getTime(),
+                        sid: 'TEST-ID',
+                    },
+                });
+
+                mpInstance._SessionManager.endSession();
+
+                // At exactly 30 minutes, session should be expired
+                expect(mpInstance._Store.sessionId).to.equal(null);


@jaissica12 let's test this in case the robot is telling the truth

src/sessionManager.ts

alexs-mparticle · 2025-12-19T15:27:03Z

src/sessionManager.ts

-                    messageType: Types.MessageType.SessionEnd,
-                });
-
-                mpInstance._Store.sessionStartDate = null;


I noticed we're not including this line in the performSessionEnd function. Can it be used there? If not, can it be safely removed?

@jaissica12 Did you address this?

looks like she added it to nullify session which happens inside of performSessionEnd

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/src/tests-session-manager.ts

src/sessionManager.ts

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/sessionManager.ts

remove trailing whitespace Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

alexs-mparticle · 2026-01-05T18:21:30Z

test/src/tests-session-manager.ts

                expect(timerSpy.getCalls().length).to.equal(1);
+
+                // Clean up stub
+                getPersistenceStub.restore();


Can we move this into an afterEach? Not a blocker.

alexs-mparticle · 2026-01-05T18:21:54Z

src/sessionManager.ts

-                    messageType: Types.MessageType.SessionEnd,
-                });
-
-                mpInstance._Store.sessionStartDate = null;


@jaissica12 Did you address this?

rmi22186 · 2026-01-05T18:30:33Z

src/sessionManager.ts

+            const { dateLastEventSent, SDKConfig } = mpInstance._Store;
+            const { sessionTimeout } = SDKConfig;
+
+            if (hasSessionTimedOut(dateLastEventSent.getTime(), sessionTimeout)) {


Unlikely, but dateLastEventSent is nullable, and so we should guard against that.

Suggested change

if (hasSessionTimedOut(dateLastEventSent.getTime(), sessionTimeout)) {

if (hasSessionTimedOut(dateLastEventSent?.getTime(), sessionTimeout)) {

rmi22186 · 2026-01-05T18:45:02Z

src/sessionManager.ts

-                    messageType: Types.MessageType.SessionEnd,
-                });
-
-                mpInstance._Store.sessionStartDate = null;


looks like she added it to nullify session which happens inside of performSessionEnd

sonarqubecloud · 2026-01-05T19:22:12Z

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-05T19:30:59Z

src/sessionManager.ts

-            ) {
+            const { dateLastEventSent, SDKConfig } = mpInstance._Store;
+            const { sessionTimeout } = SDKConfig;
+


This refactoring changes the behavior of session timeout in the initialize() method. Previously, the code used a strictly-greater-than comparison, meaning sessions would only expire AFTER the timeout period (e.g., at 30:00.001, not at 30:00.000). The new code uses greater-than-or-equal, so sessions expire AT the timeout period. While this seems more correct and matches the behavior that endSession() originally had, this is technically a breaking change that could affect existing applications relying on the previous behavior. Consider documenting this change in release notes.

Suggested change

// Note: hasSessionTimedOut() expires sessions AT the configured timeout

// boundary (using a >= comparison), whereas older versions of initialize()

// only expired sessions AFTER the timeout period using a > comparison.

// This aligns initialize() with endSession() but is a behavioral change that

// can affect applications that relied on the previous "grace" past timeout.

chore: added helper function performSessionEnd

c9c08e0

jaissica12 marked this pull request as draft November 5, 2025 15:37

jaissica12 added 2 commits December 1, 2025 09:25

add hasSessionTimedOut helper to unify timeout logic and updated tests

952f67a

Merge remote-tracking branch 'origin/development' into refactor/SDKE-…

8dfeb98

…376-refactor-endSession-in-sessionManager

jaissica12 marked this pull request as ready for review December 1, 2025 14:39

jaissica12 requested a review from rmi22186 December 1, 2025 14:42

jaissica12 changed the base branch from development to blackout2025 December 3, 2025 17:31

jaissica12 changed the base branch from blackout2025 to development December 3, 2025 17:32

rmi22186 force-pushed the development branch 3 times, most recently from a790359 to 0ba2d13 Compare December 10, 2025 15:31

alexs-mparticle requested changes Dec 10, 2025

View reviewed changes

rmi22186 force-pushed the development branch 2 times, most recently from 905641c to b16bdd2 Compare December 10, 2025 21:07

refactor initialize, hasSessionTimedOut, performSessionEnd function

a42fbf9

jaissica12 requested a review from alexs-mparticle December 17, 2025 15:39

alexs-mparticle requested a review from Copilot December 19, 2025 14:53

Copilot started reviewing on behalf of alexs-mparticle December 19, 2025 14:53 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

alexs-mparticle requested changes Dec 19, 2025

View reviewed changes

fix stub leaks and timing races in session manager tests

1741978

jaissica12 requested review from alexs-mparticle and Copilot December 19, 2025 17:00

Copilot started reviewing on behalf of jaissica12 December 19, 2025 17:00 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

test/src/tests-session-manager.ts Outdated Show resolved Hide resolved

test/src/tests-session-manager.ts Outdated Show resolved Hide resolved

test/src/tests-session-manager.ts Outdated Show resolved Hide resolved

src/sessionManager.ts Outdated Show resolved Hide resolved

update test content in session manager tests

6039a3b

jaissica12 requested a review from Copilot December 19, 2025 17:35

Copilot started reviewing on behalf of jaissica12 December 19, 2025 17:36 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

src/sessionManager.ts Outdated Show resolved Hide resolved

src/sessionManager.ts Show resolved Hide resolved

src/sessionManager.ts Show resolved Hide resolved

Update src/sessionManager.ts

1ad176d

remove trailing whitespace Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

alexs-mparticle requested a review from Copilot January 5, 2026 18:17

Copilot started reviewing on behalf of alexs-mparticle January 5, 2026 18:17 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

alexs-mparticle reviewed Jan 5, 2026

View reviewed changes

rmi22186 reviewed Jan 5, 2026

View reviewed changes

add null check for dateLastEventSent in sessionManager

3c4a644

jaissica12 requested review from alexs-mparticle, Copilot and rmi22186 January 5, 2026 19:26

Copilot started reviewing on behalf of jaissica12 January 5, 2026 19:26 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

rmi22186 approved these changes Jan 5, 2026

View reviewed changes

alexs-mparticle approved these changes Jan 5, 2026

View reviewed changes

	if (hasSessionTimedOut(dateLastEventSent.getTime(), sessionTimeout)) {
	if (hasSessionTimedOut(dateLastEventSent?.getTime(), sessionTimeout)) {

+            // Note: hasSessionTimedOut() expires sessions AT the configured timeout
+            // boundary (using a >= comparison), whereas older versions of initialize()
+            // only expired sessions AFTER the timeout period using a > comparison.
+            // This aligns initialize() with endSession() but is a behavioral change that
+            // can affect applications that relied on the previous "grace" past timeout.

refactor: added helper function performSessionEnd #1113

Are you sure you want to change the base?

refactor: added helper function performSessionEnd #1113

Uh oh!

Conversation

jaissica12 commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

What Has Changed

Screenshots/Video

Checklist

Additional Notes

Reference Issue (For employees only. Ignore if you are an outside contributor)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

jaissica12 commented Nov 5, 2025 •

edited

Loading