startSession should no longer clone the client

packages/plugin-express/src/express.js

@@ -13,9 +13,11 @@ module.exports = {
   name: 'express',
   load: client => {
     const requestHandler = (req, res, next) => {
-      // Get a client to be scoped to this request. If sessions are enabled, use the
-      // resumeSession() call to get a session client, otherwise, clone the existing client.
-      const requestClient = client._config.autoTrackSessions ? client.resumeSession() : clone(client)
+      // clone the client to be scoped to this request. If sessions are enabled, start one
+      const requestClient = clone(client)
+      if (requestClient._config.autoTrackSessions) {
+        requestClient.startSession()
+      }
 
       // attach it to the request
       req.bugsnag = requestClient

packages/plugin-koa/src/koa.js

@@ -14,9 +14,11 @@ module.exports = {
   name: 'koa',
   load: client => {
     const requestHandler = async (ctx, next) => {
-      // Get a client to be scoped to this request. If sessions are enabled, use the
-      // resumeSession() call to get a session client, otherwise, clone the existing client.
-      const requestClient = client._config.autoTrackSessions ? client.resumeSession() : clone(client)
+      // clone the client to be scoped to this request. If sessions are enabled, start one
+      const requestClient = clone(client)
+      if (requestClient._config.autoTrackSessions) {
+        requestClient.startSession()
+      }
 
       ctx.bugsnag = requestClient
 
@@ -35,9 +37,11 @@ module.exports = {
     }
 
     requestHandler.v1 = function * (next) {
-      // Get a client to be scoped to this request. If sessions are enabled, use the
-      // resumeSession() call to get a session client, otherwise, clone the existing client.
-      const requestClient = client._config.autoTrackSessions ? client.resumeSession() : clone(client)
+      // clone the client to be scoped to this request. If sessions are enabled, start one
+      const requestClient = clone(client)
+      if (requestClient._config.autoTrackSessions) {
+        requestClient.startSession()
+      }
 
       this.bugsnag = requestClient
 

packages/plugin-koa/test/koa.test.ts

@@ -34,7 +34,7 @@ describe('plugin: koa', () => {
   })
 
   describe('requestHandler', () => {
-    it('should start a session and attach a client to the context', async () => {
+    it('should clone the client, start a session and attach the cloned client to the context', async () => {
       const client = new Client({ apiKey: 'api_key', plugins: [plugin] })
 
       const startSession = jest.fn().mockReturnValue(client)
@@ -58,10 +58,11 @@ describe('plugin: koa', () => {
 
       expect(client._logger.warn).not.toHaveBeenCalled()
       expect(client._logger.error).not.toHaveBeenCalled()
-      expect(startSession).not.toHaveBeenCalled()
+      expect(startSession).toHaveBeenCalledTimes(1)
       expect(pauseSession).not.toHaveBeenCalled()
-      expect(resumeSession).toHaveBeenCalledTimes(1)
-      expect(context.bugsnag).toBe(client)
+      expect(resumeSession).not.toHaveBeenCalled()
+      expect(context.bugsnag).toStrictEqual(expect.any(Client))
+      expect(context.bugsnag).not.toBe(client)
       expect(client._clientContext.run).toHaveBeenCalledWith(expect.any(Client), next)
     })
 
@@ -114,7 +115,7 @@ describe('plugin: koa', () => {
       expect(client._clientContext.run).toHaveBeenCalledWith(expect.any(Client), next)
 
       const event: Event = await new Promise(resolve => {
-        client.notify(new Error('abc'), noop, (_, event) => resolve(event as Event))
+        (context.bugsnag as Client).notify(new Error('abc'), noop, (_, event) => resolve(event as Event))
       })
 
       expect(client._logger.warn).not.toHaveBeenCalled()
@@ -183,7 +184,7 @@ describe('plugin: koa', () => {
       expect(client._clientContext.run).toHaveBeenCalledWith(expect.any(Client), next)
 
       const event: Event = await new Promise(resolve => {
-        client.notify(new Error('abc'), noop, (_, event) => resolve(event as Event))
+        (context.bugsnag as Client).notify(new Error('abc'), noop, (_, event) => resolve(event as Event))
       })
 
       expect(client._logger.warn).not.toHaveBeenCalled()

packages/plugin-restify/src/restify.js

@@ -13,9 +13,11 @@ module.exports = {
   name: 'restify',
   load: client => {
     const requestHandler = (req, res, next) => {
-      // Get a client to be scoped to this request. If sessions are enabled, use the
-      // resumeSession() call to get a session client, otherwise, clone the existing client.
-      const requestClient = client._config.autoTrackSessions ? client.resumeSession() : clone(client)
+      // clone the client to be scoped to this request. If sessions are enabled, start one
+      const requestClient = clone(client)
+      if (requestClient._config.autoTrackSessions) {
+        requestClient.startSession()
+      }
 
       // attach it to the request
       req.bugsnag = requestClient

packages/plugin-server-session/session.js

@@ -1,5 +1,4 @@
 const intRange = require('@bugsnag/core/lib/validators/int-range')
-const clone = require('@bugsnag/core/lib/clone-client')
 const SessionTracker = require('./tracker')
 const Backoff = require('backo')
 const runSyncCallbacks = require('@bugsnag/core/lib/sync-callback-runner')
@@ -11,11 +10,10 @@ module.exports = {
     sessionTracker.start()
     client._sessionDelegate = {
       startSession: (client, session) => {
-        const sessionClient = clone(client)
-        sessionClient._session = session
-        sessionClient._pausedSession = null
-        sessionTracker.track(sessionClient._session)
-        return sessionClient
+        client._session = session
+        client._pausedSession = null
+        sessionTracker.track(client._session)
+        return client
       },
       pauseSession: (client) => {
         client._pausedSession = client._session
@@ -31,7 +29,6 @@ module.exports = {
         if (client._pausedSession) {
           client._session = client._pausedSession
           client._pausedSession = null
-
           return client
         }
 

packages/plugin-server-session/test/session.test.ts

@@ -96,30 +96,6 @@ describe('plugin: server sessions', () => {
     c.startSession()
   })
 
-  it('should clone properties that shouldn’t be mutated on the original client', () => {
-    class TrackerMock extends EventEmitter {
-      start () {}
-      stop () {}
-      track () {}
-    }
-    Tracker.mockImplementation(() => new TrackerMock() as any)
-
-    const c = new Client({ apiKey: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' }, undefined, [plugin])
-
-    c.leaveBreadcrumb('tick')
-    c._metadata = { datetime: { tz: 'GMT+1' } }
-
-    const sessionClient = c.startSession()
-
-    sessionClient.leaveBreadcrumb('tock')
-    sessionClient.addMetadata('other', { widgetsAdded: 'cat,dog,mouse' })
-
-    expect(c._breadcrumbs.length).toBe(1)
-    expect(Object.keys(c._metadata).length).toBe(1)
-    expect(sessionClient._breadcrumbs.length).toBe(2)
-    expect(Object.keys(sessionClient._metadata).length).toBe(2)
-  })
-
   it('should support pausing/resuming sessions', () => {
     class TrackerMock extends EventEmitter {
       start () {}
@@ -131,27 +107,27 @@ describe('plugin: server sessions', () => {
     const c = new Client({ apiKey: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' }, undefined, [plugin])
 
     // start a session and get its id
-    const sessionClient = c.startSession()
-    const sid0 = (sessionClient._session as Session).id
+    c.startSession()
+    const sid0 = (c._session as Session).id
 
     // ensure pausing the session clears the client._session property
-    sessionClient.pauseSession()
-    const s1 = sessionClient._session
-    const psid1 = (sessionClient._pausedSession as Session).id
+    c.pauseSession()
+    const s1 = c._session
+    const psid1 = (c._pausedSession as Session).id
     expect(s1).toBe(null)
     expect(psid1).toBe(sid0)
 
     // ensure resuming the session gets back the original session (not a new one)
-    sessionClient.resumeSession()
-    const sid2 = (sessionClient._session as Session).id
+    c.resumeSession()
+    const sid2 = (c._session as Session).id
     expect(sid2).toBe(sid0)
 
     // ensure resumeSession() starts a new one when no paused session exists
-    sessionClient._session = null
-    sessionClient._pausedSession = null
-    const resumedClient = sessionClient.resumeSession()
-    expect(resumedClient._session).toBeTruthy()
-    const sid3 = (resumedClient._session as Session).id
+    c._session = null
+    c._pausedSession = null
+    c.resumeSession()
+    expect(c._session).toBeTruthy()
+    const sid3 = (c._session as unknown as Session).id
     expect(sid3).not.toBe(sid0)
   })
 })