Fix bug where session ids weren't stored when a session timed out.

amplitude · Dec 20, 2014 · 86f5b46 · 86f5b46
1 parent aefe165
commit 86f5b46
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 17 deletions.
diff --git a/amplitude.js b/amplitude.js
@@ -324,15 +324,13 @@ Amplitude.prototype.logEvent = function(eventType, eventProperties) {
   try {
     var eventTime = new Date().getTime();
     var eventId = this.nextEventId();
-    var sessionId = this._sessionId;
-    var lastEventTime = this._lastEventTime;
     var ua = this._ua;
-    if (!sessionId || !lastEventTime || eventTime - lastEventTime > this.options.sessionTimeout) {
-      sessionId = eventTime;
-      localStorage.setItem(LocalStorageKeys.SESSION_ID, sessionId);
+    if (!this._sessionId || !this._lastEventTime || eventTime - this._lastEventTime > this.options.sessionTimeout) {
+      this._sessionId = eventTime;
+      localStorage.setItem(LocalStorageKeys.SESSION_ID, this._sessionId);
     }
-    lastEventTime = eventTime;
-    localStorage.setItem(LocalStorageKeys.LAST_EVENT_TIME, lastEventTime);
+    this._lastEventTime = eventTime;
+    localStorage.setItem(LocalStorageKeys.LAST_EVENT_TIME, this._lastEventTime);
     localStorage.setItem(LocalStorageKeys.LAST_EVENT_ID, eventId);
 
     eventProperties = eventProperties || {};
@@ -341,7 +339,7 @@ Amplitude.prototype.logEvent = function(eventType, eventProperties) {
       user_id: this.options.userId || this.options.deviceId,
       timestamp: eventTime,
       event_id: eventId,
-      session_id: sessionId || -1,
+      session_id: this._sessionId || -1,
       event_type: eventType,
       version_name: this.options.versionName || null,
       platform: this.options.platform,

diff --git a/amplitude.min.js b/amplitude.min.js
diff --git a/src/amplitude.js b/src/amplitude.js
@@ -214,15 +214,13 @@ Amplitude.prototype.logEvent = function(eventType, eventProperties) {
   try {
     var eventTime = new Date().getTime();
     var eventId = this.nextEventId();
-    var sessionId = this._sessionId;
-    var lastEventTime = this._lastEventTime;
     var ua = this._ua;
-    if (!sessionId || !lastEventTime || eventTime - lastEventTime > this.options.sessionTimeout) {
-      sessionId = eventTime;
-      localStorage.setItem(LocalStorageKeys.SESSION_ID, sessionId);
+    if (!this._sessionId || !this._lastEventTime || eventTime - this._lastEventTime > this.options.sessionTimeout) {
+      this._sessionId = eventTime;
+      localStorage.setItem(LocalStorageKeys.SESSION_ID, this._sessionId);
     }
-    lastEventTime = eventTime;
-    localStorage.setItem(LocalStorageKeys.LAST_EVENT_TIME, lastEventTime);
+    this._lastEventTime = eventTime;
+    localStorage.setItem(LocalStorageKeys.LAST_EVENT_TIME, this._lastEventTime);
     localStorage.setItem(LocalStorageKeys.LAST_EVENT_ID, eventId);
 
     eventProperties = eventProperties || {};
@@ -231,7 +229,7 @@ Amplitude.prototype.logEvent = function(eventType, eventProperties) {
       user_id: this.options.userId || this.options.deviceId,
       timestamp: eventTime,
       event_id: eventId,
-      session_id: sessionId || -1,
+      session_id: this._sessionId || -1,
       event_type: eventType,
       version_name: this.options.versionName || null,
       platform: this.options.platform,

diff --git a/test/amplitude.js b/test/amplitude.js
@@ -167,4 +167,31 @@ describe('Amplitude', function() {
       assert.deepEqual(events[0].event_properties, {prop: true});
     });
   });
+
+  describe('sessionId', function() {
+
+    var clock;
+
+    beforeEach(function() {
+      clock = sinon.useFakeTimers();
+      amplitude.init(apiKey);
+    });
+
+    afterEach(function() {
+      reset();
+      clock.restore();
+    });
+
+    it('should create new session IDs on timeout', function() {
+      var sessionId = amplitude._sessionId;
+      clock.tick(30 * 60 * 1000 + 1);
+      amplitude.logEvent('Event Type 1');
+      assert.lengthOf(server.requests, 1);
+      var events = JSON.parse(querystring.parse(server.requests[0].requestBody).e);
+      assert.equal(events.length, 1);
+      assert.notEqual(events[0].session_id, sessionId);
+      assert.notEqual(amplitude._sessionId, sessionId);
+      assert.equal(events[0].session_id, amplitude._sessionId);
+    });
+  });
 });