fix: Clear activeTransaction from the scope and always start idle timers

getsentry · Jan 28, 2021 · d52947b · d52947b
1 parent 8c20fd2
commit d52947b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 18 deletions.
diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts
@@ -209,8 +209,8 @@ export class BrowserTracing implements Integration {
     }
 
     const hub = this._getCurrentHub();
-    const idleTransaction = startIdleTransaction(hub, finalContext, idleTimeout, true);
     logger.log(`[Tracing] Starting ${finalContext.op} transaction on scope`);
+    const idleTransaction = startIdleTransaction(hub, finalContext, idleTimeout, true);
     idleTransaction.registerBeforeFinishCallback((transaction, endTimestamp) => {
       this._metrics.addPerformanceEntries(transaction);
       adjustTransactionDuration(secToMs(maxTransactionDuration), transaction, endTimestamp);

diff --git a/packages/tracing/src/idletransaction.ts b/packages/tracing/src/idletransaction.ts
@@ -93,6 +93,12 @@ export class IdleTransaction extends Transaction {
       logger.log(`Setting idle transaction on scope. Span ID: ${this.spanId}`);
       _idleHub.configureScope(scope => scope.setSpan(this));
     }
+
+    this._initTimeout = setTimeout(() => {
+      if (!this._finished) {
+        this.finish();
+      }
+    }, this._idleTimeout);
   }
 
   /** {@inheritDoc} */
@@ -130,16 +136,16 @@ export class IdleTransaction extends Transaction {
         return keepSpan;
       });
 
-      // this._onScope is true if the transaction was previously on the scope.
-      if (this._onScope) {
-        clearActiveTransaction(this._idleHub);
-      }
-
       logger.log('[Tracing] flushing IdleTransaction');
     } else {
       logger.log('[Tracing] No active IdleTransaction');
     }
 
+    // this._onScope is true if the transaction was previously on the scope.
+    if (this._onScope) {
+      clearActiveTransaction(this._idleHub);
+    }
+
     return super.finish(endTimestamp);
   }
 
@@ -159,12 +165,6 @@ export class IdleTransaction extends Transaction {
    */
   public initSpanRecorder(maxlen?: number): void {
     if (!this.spanRecorder) {
-      this._initTimeout = setTimeout(() => {
-        if (!this._finished) {
-          this.finish();
-        }
-      }, this._idleTimeout);
-
       const pushActivity = (id: string): void => {
         if (this._finished) {
           return;

diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts
@@ -150,7 +150,7 @@ describe('IdleTransaction', () => {
       const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, 1000);
       transaction.initSpanRecorder(10);
 
-      jest.runTimersToTime(DEFAULT_IDLE_TIMEOUT);
+      jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT);
       expect(transaction.endTimestamp).toBeDefined();
     });
 
@@ -159,28 +159,34 @@ describe('IdleTransaction', () => {
       transaction.initSpanRecorder(10);
       transaction.startChild({});
 
-      jest.runTimersToTime(DEFAULT_IDLE_TIMEOUT);
+      jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT);
       expect(transaction.endTimestamp).toBeUndefined();
     });
   });
 
   describe('heartbeat', () => {
     it('does not start heartbeat if there is no span recorder', () => {
-      const transaction = new IdleTransaction({ name: 'foo' }, hub, 1000);
+      const HEARTBEAT_INTERVAL = 5000;
+      // 20s to exceed 3 heartbeats
+      const transaction = new IdleTransaction({ name: 'foo' }, hub, 20000);
       const mockFinish = jest.spyOn(transaction, 'finish');
 
+      expect(transaction.status).not.toEqual(SpanStatus.DeadlineExceeded);
       expect(mockFinish).toHaveBeenCalledTimes(0);
 
       // Beat 1
-      jest.runOnlyPendingTimers();
+      jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
+      expect(transaction.status).not.toEqual(SpanStatus.DeadlineExceeded);
       expect(mockFinish).toHaveBeenCalledTimes(0);
 
       // Beat 2
-      jest.runOnlyPendingTimers();
+      jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
+      expect(transaction.status).not.toEqual(SpanStatus.DeadlineExceeded);
       expect(mockFinish).toHaveBeenCalledTimes(0);
 
       // Beat 3
-      jest.runOnlyPendingTimers();
+      jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
+      expect(transaction.status).not.toEqual(SpanStatus.DeadlineExceeded);
       expect(mockFinish).toHaveBeenCalledTimes(0);
     });