Skip to content

Commit

Permalink
Keep the session listeners alive after the call object destroyed (#1160)
Browse files Browse the repository at this point in the history
* Fix session emitter

* setup ws client with fabric

* fix build

* fix root saga tests

* fix incoming call manager test

* fix conversation tests

* setup playground to have connect and dial

* fix the playgroud

* remove unused connect

* fix conversation test

* remove emitter from the client options

* include the WSClient contract

* ensure proper session storage cleanup

* include the http client contract

* add a few comments

* fix unit tests

* fix unit tests

* add a cleanup test for cf sdk

* add e2e tests for the video sdk

* explicit on socket destroy call

* fix the wss url assert

* signleton SignalWire client

* include changeset
  • Loading branch information
iAmmar7 authored Jan 21, 2025
1 parent 63849f9 commit fd39f12
Show file tree
Hide file tree
Showing 56 changed files with 1,744 additions and 1,065 deletions.
9 changes: 9 additions & 0 deletions .changeset/stupid-bobcats-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@signalwire/realtime-api': patch
'@signalwire/core': patch
'@signalwire/js': patch
---

- Fix session emitter
- Make SignalWire a singelton for Call Fabric SDK
- Fix memory leak
1 change: 1 addition & 0 deletions internal/e2e-js/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const reattachTests = [
]
const callfabricTests = [
'address.spec.ts',
'cleanup.spec.ts',
'conversation.spec.ts',
'raiseHand.spec.ts',
'reattach.spec.ts',
Expand Down
194 changes: 194 additions & 0 deletions internal/e2e-js/tests/callfabric/cleanup.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
import { uuid } from '@signalwire/core'
import { test, expect } from '../../fixtures'
import {
createCFClient,
dialAddress,
disconnectClient,
leaveRoom,
SERVER_URL,
} from '../../utils'

test.describe('Clean up', () => {
test('it should create a webscoket client', async ({ createCustomPage }) => {
const page = await createCustomPage({ name: '[page]' })
await page.goto(SERVER_URL)

let websocketUrl: string | null = null
let websocketClosed = false

// A promise to wait for the WebSocket close event
const waitForWebSocketClose = new Promise<void>((resolve) => {
page.on('websocket', (ws) => {
websocketUrl = ws.url()

ws.on('close', () => {
websocketClosed = true
resolve()
})
})
})

expect(websocketUrl).toBe(null)

await createCFClient(page)

await disconnectClient(page)

await waitForWebSocketClose
expect(websocketUrl).toBeTruthy()
expect(websocketUrl).toContain('wss://')
expect(websocketClosed).toBeTruthy()
})

test('it should cleanup session emitter and workers', async ({
createCustomPage,
}) => {
const page = await createCustomPage({ name: '[page]' })
await page.goto(SERVER_URL)

await createCFClient(page, { attachSagaMonitor: true })

await test.step('the client should have workers and listeners attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const client = window._client

return {
clientListenersLength: client.__wsClient.sessionEventNames().length,
clientWorkersLength: client.__wsClient._runningWorkers.length,
// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.clientWorkersLength).toBeGreaterThan(0)
expect(watchers.globalWorkersLength).toBeGreaterThan(0)
})

await disconnectClient(page)

await test.step('the client should not have workers and listeners attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const client = window._client

return {
clientListenersLength: client.__wsClient.sessionEventNames().length,
clientWorkersLength: client.__wsClient._runningWorkers.length,
// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.clientListenersLength).toBe(0)
expect(watchers.clientWorkersLength).toBe(0)
expect(watchers.globalWorkersLength).toBe(0)
})
})

test('it should cleanup call emitter and workers without affecting the client', async ({
createCustomPage,
resource,
}) => {
const page = await createCustomPage({ name: '[page]' })
await page.goto(SERVER_URL)

const roomName = `e2e-cleanup_${uuid()}`
await resource.createVideoRoomResource(roomName)

await createCFClient(page, { attachSagaMonitor: true })

// Dial an address and join a video room
await dialAddress(page, {
address: `/public/${roomName}?channel=video`,
})

const { beforeClientListenersLength, beforeClientWorkersLength } =
await test.step('call and client should have watchers attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const client = window._client

return {
clientListenersLength: client.__wsClient.sessionEventNames().length,
clientWorkersLength: client.__wsClient._runningWorkers.length,

// @ts-expect-error
callListeners: window._roomObj.eventNames().length,
// @ts-expect-error
callWorkersLength: window._roomObj._runningWorkers.length,

// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.clientListenersLength).toBeGreaterThan(0)
expect(watchers.clientWorkersLength).toBeGreaterThan(0)
expect(watchers.callListeners).toBeGreaterThan(0)
expect(watchers.callWorkersLength).toBeGreaterThan(0)
expect(watchers.globalWorkersLength).toBeGreaterThan(0)

return {
beforeClientListenersLength: watchers.clientListenersLength,
beforeClientWorkersLength: watchers.clientWorkersLength,
}
})

await leaveRoom(page)

await test.step('call should not have any watchers attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const client = window._client

return {
clientListenersLength: client.__wsClient.sessionEventNames().length,
clientWorkersLength: client.__wsClient._runningWorkers.length,

// @ts-expect-error
callListeners: window._roomObj.eventNames().length,
// @ts-expect-error
callWorkersLength: window._roomObj._runningWorkers.length,

// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.clientListenersLength).toBe(beforeClientListenersLength)
expect(watchers.clientWorkersLength).toBe(beforeClientWorkersLength)
expect(watchers.callListeners).toBe(0)
expect(watchers.callWorkersLength).toBe(0)
expect(watchers.globalWorkersLength).toBeGreaterThan(0)
})

await disconnectClient(page)

await test.step('client should not have any watchers attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const client = window._client

return {
clientListenersLength: client.__wsClient.sessionEventNames().length,
clientWorkersLength: client.__wsClient._runningWorkers.length,

// @ts-expect-error
callListeners: window._roomObj.eventNames().length,
// @ts-expect-error
callWorkersLength: window._roomObj._runningWorkers.length,

// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.clientListenersLength).toBe(0)
expect(watchers.clientWorkersLength).toBe(0)
expect(watchers.callListeners).toBe(0)
expect(watchers.callWorkersLength).toBe(0)
expect(watchers.globalWorkersLength).toBe(0)
})
})
})
1 change: 0 additions & 1 deletion internal/e2e-js/tests/callfabric/conversation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ test.describe('Conversation Room', () => {
return new Promise<ConversationMessageEventParams>(async (resolve) => {
// @ts-expect-error
const client: SignalWireClient = window._client
await client.connect()
client.conversation.subscribe(resolve)
const result = await client.conversation.getConversations()
const convo = result.data.filter((c) => c.id == addressId)[0]
Expand Down
72 changes: 72 additions & 0 deletions internal/e2e-js/tests/roomSessionCleanup.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { test, expect } from '../fixtures'
import {
createTestRoomSession,
expectRoomJoined,
leaveRoom,
randomizeRoomName,
SERVER_URL,
} from '../utils'

test.describe('RoomSession', () => {
test('it should join the room and then leave with cleanup', async ({
createCustomPage,
}) => {
const page = await createCustomPage({ name: '[page]' })
await page.goto(SERVER_URL)

const roomName = randomizeRoomName('e2e-cleanup')
await createTestRoomSession(page, {
vrt: {
room_name: roomName,
user_name: 'e2e_test',
auto_create_room: true,
},
initialEvents: [],
attachSagaMonitor: true,
})

await expectRoomJoined(page)

await test.step('the room should have workers and listeners attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const roomObj = window._roomObj

return {
roomSessionListenersLength: roomObj.sessionEventNames().length,
roomListenersLength: roomObj.eventNames().length,
roomWorkersLength: roomObj._runningWorkers.length,
// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.roomSessionListenersLength).toBeGreaterThan(0)
expect(watchers.roomListenersLength).toBeGreaterThan(0)
expect(watchers.roomWorkersLength).toBeGreaterThan(0)
expect(watchers.globalWorkersLength).toBeGreaterThan(0)
})

await leaveRoom(page)

await test.step('the room should not have any workers and listeners attached', async () => {
const watchers: Record<string, number> = await page.evaluate(() => {
// @ts-expect-error
const roomObj = window._roomObj

return {
roomSessionListenersLength: roomObj.sessionEventNames().length,
roomListenersLength: roomObj.eventNames().length,
roomWorkersLength: roomObj._runningWorkers.length,
// @ts-expect-error
globalWorkersLength: window._runningWorkers.length,
}
})

expect(watchers.roomSessionListenersLength).toBe(0)
expect(watchers.roomListenersLength).toBe(0)
expect(watchers.roomWorkersLength).toBe(0)
expect(watchers.globalWorkersLength).toBe(0)
})
})
})
Loading

0 comments on commit fd39f12

Please sign in to comment.