Skip to content

Commit a9b90bc

Browse files
committed
fix(user_status): stop the heartbeat firing on every mouse movement
The away countdown was cancelled by passing the callback function to clearTimeout() instead of the id returned by setTimeout(), so the clear did nothing and the id was thrown away. Every burst of mouse movement scheduled another two minute timer that nothing could cancel, and each one that expired marked an active user as away. The next movement flipped them back and sent a heartbeat, so an ordinary browsing session sent around 600 heartbeats an hour instead of the 13 the code intends. The scheduling now lives in its own module, which makes it testable without mounting the component and gives the timers a single owner. That also fixes the teardown: the listener was registered as "mousemove" with capture and removed as "mouseMove" without it, so it was never actually removed and every unmounted component leaked one. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent 75e10ec commit a9b90bc

5 files changed

Lines changed: 183 additions & 36 deletions

File tree

apps/user_status/src/UserStatus.vue

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@
4040
<script>
4141
import { getCurrentUser } from '@nextcloud/auth'
4242
import { subscribe, unsubscribe } from '@nextcloud/event-bus'
43-
import debounce from 'debounce'
4443
import { defineAsyncComponent } from 'vue'
4544
import NcButton from '@nextcloud/vue/components/NcButton'
4645
import NcListItem from '@nextcloud/vue/components/NcListItem'
4746
import NcUserStatusIcon from '@nextcloud/vue/components/NcUserStatusIcon'
4847
import { logger } from './logger.ts'
4948
import OnlineStatusMixin from './mixins/OnlineStatusMixin.js'
49+
import { startHeartbeat } from './services/heartbeatScheduler.ts'
5050
import { sendHeartbeat } from './services/heartbeatService.js'
5151
5252
export default {
@@ -75,11 +75,8 @@ export default {
7575
7676
data() {
7777
return {
78-
heartbeatInterval: null,
79-
isAway: false,
8078
isModalOpen: false,
81-
mouseMoveListener: null,
82-
setAwayTimeout: null,
79+
stopHeartbeat: null,
8380
}
8481
},
8582
@@ -91,31 +88,7 @@ export default {
9188
this.$store.dispatch('loadStatusFromInitialState')
9289
9390
if (OC.config.session_keepalive) {
94-
// Send the latest status to the server every 5 minutes
95-
this.heartbeatInterval = setInterval(this._backgroundHeartbeat.bind(this), 1000 * 60 * 5)
96-
this.setAwayTimeout = () => {
97-
this.isAway = true
98-
}
99-
// Catch mouse movements, but debounce to once every 30 seconds
100-
this.mouseMoveListener = debounce(() => {
101-
const wasAway = this.isAway
102-
this.isAway = false
103-
// Reset the two minute counter
104-
clearTimeout(this.setAwayTimeout)
105-
// If the user did not move the mouse within two minutes,
106-
// mark them as away
107-
setTimeout(this.setAwayTimeout, 1000 * 60 * 2)
108-
109-
if (wasAway) {
110-
this._backgroundHeartbeat()
111-
}
112-
}, 1000 * 2, { immediate: true })
113-
window.addEventListener('mousemove', this.mouseMoveListener, {
114-
capture: true,
115-
passive: true,
116-
})
117-
118-
this._backgroundHeartbeat()
91+
this.stopHeartbeat = startHeartbeat((isAway) => this._backgroundHeartbeat(isAway))
11992
}
12093
subscribe('user_status:status.updated', this.handleUserStatusUpdated)
12194
},
@@ -124,8 +97,7 @@ export default {
12497
* Some housekeeping before destroying the component
12598
*/
12699
beforeUnmount() {
127-
window.removeEventListener('mouseMove', this.mouseMoveListener)
128-
clearInterval(this.heartbeatInterval)
100+
this.stopHeartbeat?.()
129101
unsubscribe('user_status:status.updated', this.handleUserStatusUpdated)
130102
},
131103
@@ -147,12 +119,13 @@ export default {
147119
/**
148120
* Sends the status heartbeat to the server
149121
*
122+
* @param {boolean} isAway Whether the user is currently away
150123
* @return {Promise<void>}
151124
* @private
152125
*/
153-
async _backgroundHeartbeat() {
126+
async _backgroundHeartbeat(isAway) {
154127
try {
155-
const status = await sendHeartbeat(this.isAway)
128+
const status = await sendHeartbeat(isAway)
156129
if (status?.userId) {
157130
this.$store.dispatch('setStatusFromHeartbeat', status)
158131
} else {
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
7+
import {
8+
AWAY_TIMEOUT,
9+
HEARTBEAT_INTERVAL,
10+
MOUSE_MOVE_DEBOUNCE,
11+
startHeartbeat,
12+
} from './heartbeatScheduler.ts'
13+
14+
const HOUR = 60 * 60 * 1000
15+
16+
let stop: (() => void) | undefined
17+
18+
/**
19+
* Move the mouse for a second, then hold still.
20+
*
21+
* @param gap - Milliseconds of stillness after the burst
22+
*/
23+
async function moveThenRest(gap: number): Promise<void> {
24+
for (let i = 0; i < 10; i++) {
25+
window.dispatchEvent(new MouseEvent('mousemove'))
26+
await vi.advanceTimersByTimeAsync(100)
27+
}
28+
await vi.advanceTimersByTimeAsync(gap)
29+
}
30+
31+
describe('heartbeat scheduler', () => {
32+
beforeAll(() => {
33+
// `debounce` compares Date.now() against its own timestamp, so Date has to stay faked alongside the timers
34+
vi.useFakeTimers()
35+
})
36+
37+
beforeEach(() => {
38+
vi.clearAllTimers()
39+
vi.resetAllMocks()
40+
})
41+
42+
afterEach(() => {
43+
stop?.()
44+
stop = undefined
45+
})
46+
47+
it('sends a heartbeat on start', () => {
48+
const beat = vi.fn()
49+
stop = startHeartbeat(beat)
50+
51+
expect(beat).toHaveBeenCalledTimes(1)
52+
expect(beat).toHaveBeenCalledWith(false)
53+
})
54+
55+
it('sends a heartbeat every five minutes', async () => {
56+
const beat = vi.fn()
57+
stop = startHeartbeat(beat)
58+
59+
await vi.advanceTimersByTimeAsync(HEARTBEAT_INTERVAL - 1000)
60+
expect(beat).toHaveBeenCalledTimes(1)
61+
62+
await vi.advanceTimersByTimeAsync(1000)
63+
expect(beat).toHaveBeenCalledTimes(2)
64+
65+
await vi.advanceTimersByTimeAsync(HEARTBEAT_INTERVAL)
66+
expect(beat).toHaveBeenCalledTimes(3)
67+
})
68+
69+
it('does not send extra heartbeats while the user keeps moving the mouse', async () => {
70+
const beat = vi.fn()
71+
stop = startHeartbeat(beat)
72+
73+
const cycle = 1000 + 5000
74+
for (let elapsed = 0; elapsed < HOUR; elapsed += cycle) {
75+
await moveThenRest(5000)
76+
}
77+
78+
expect(beat).toHaveBeenCalledTimes(1 + HOUR / HEARTBEAT_INTERVAL)
79+
// the away countdown is restarted, never accumulated
80+
expect(vi.getTimerCount()).toBeLessThanOrEqual(3)
81+
})
82+
83+
it('reports the user as away after two minutes without mouse movement', async () => {
84+
const beat = vi.fn()
85+
stop = startHeartbeat(beat)
86+
87+
window.dispatchEvent(new MouseEvent('mousemove'))
88+
await vi.advanceTimersByTimeAsync(AWAY_TIMEOUT + 1000)
89+
90+
await vi.advanceTimersByTimeAsync(HEARTBEAT_INTERVAL - AWAY_TIMEOUT - 1000)
91+
expect(beat).toHaveBeenLastCalledWith(true)
92+
})
93+
94+
it('sends one heartbeat when the user comes back from being away', async () => {
95+
const beat = vi.fn()
96+
stop = startHeartbeat(beat)
97+
98+
window.dispatchEvent(new MouseEvent('mousemove'))
99+
await vi.advanceTimersByTimeAsync(AWAY_TIMEOUT + MOUSE_MOVE_DEBOUNCE)
100+
const beforeReturn = beat.mock.calls.length
101+
102+
window.dispatchEvent(new MouseEvent('mousemove'))
103+
expect(beat).toHaveBeenCalledTimes(beforeReturn + 1)
104+
expect(beat).toHaveBeenLastCalledWith(false)
105+
})
106+
107+
it('stops the interval, the away countdown and the mouse listener', async () => {
108+
const beat = vi.fn()
109+
const stopHeartbeat = startHeartbeat(beat)
110+
window.dispatchEvent(new MouseEvent('mousemove'))
111+
112+
stopHeartbeat()
113+
114+
await vi.advanceTimersByTimeAsync(3 * HEARTBEAT_INTERVAL)
115+
window.dispatchEvent(new MouseEvent('mousemove'))
116+
await vi.advanceTimersByTimeAsync(3 * HEARTBEAT_INTERVAL)
117+
118+
expect(beat).toHaveBeenCalledTimes(1)
119+
expect(vi.getTimerCount()).toBe(0)
120+
})
121+
})
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import debounce from 'debounce'
7+
8+
/** Has to stay below the server margin between `StatusService::REFRESH_STATUS_THRESHOLD` and `StatusService::INVALIDATE_STATUS_THRESHOLD`. */
9+
export const HEARTBEAT_INTERVAL = 5 * 60 * 1000
10+
11+
export const AWAY_TIMEOUT = 2 * 60 * 1000
12+
13+
export const MOUSE_MOVE_DEBOUNCE = 2 * 1000
14+
15+
/**
16+
* Send heartbeats on a fixed interval, and once more whenever the user comes back from being away.
17+
*
18+
* @param beat - Called with the current away state when a heartbeat is due
19+
* @return Function that stops the heartbeat and removes every timer and listener
20+
*/
21+
export function startHeartbeat(beat: (isAway: boolean) => void): () => void {
22+
let isAway = false
23+
let awayTimeout: ReturnType<typeof setTimeout> | undefined
24+
25+
const onMouseMove = debounce(() => {
26+
const wasAway = isAway
27+
isAway = false
28+
29+
clearTimeout(awayTimeout)
30+
awayTimeout = setTimeout(() => {
31+
isAway = true
32+
}, AWAY_TIMEOUT)
33+
34+
if (wasAway) {
35+
beat(isAway)
36+
}
37+
}, MOUSE_MOVE_DEBOUNCE, { immediate: true })
38+
39+
const interval = setInterval(() => beat(isAway), HEARTBEAT_INTERVAL)
40+
window.addEventListener('mousemove', onMouseMove, {
41+
capture: true,
42+
passive: true,
43+
})
44+
45+
beat(isAway)
46+
47+
return () => {
48+
clearInterval(interval)
49+
clearTimeout(awayTimeout)
50+
onMouseMove.clear()
51+
window.removeEventListener('mousemove', onMouseMove, { capture: true })
52+
}
53+
}

dist/user_status-menu.mjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

dist/user_status-menu.mjs.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)