Clean up rate limit window listeners

This commit is contained in:
Neil
2026-05-30 13:14:02 -07:00
committed by GitHub
parent d67d8defa5
commit 03fe971d9f
2 changed files with 74 additions and 8 deletions
+59
View File
@@ -3,6 +3,7 @@ stale-data handling, account-switch generation, and OpenCode config-change
semantics covered in service.ts, which already carries the same pragma.
Keeping them in one file makes the ordering contract reviewable as a unit. */
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { EventEmitter } from 'events'
import type { ProviderRateLimits } from '../../shared/rate-limit-types'
import { RateLimitService } from './service'
import { fetchClaudeRateLimits, fetchManagedAccountUsage } from './claude-fetcher'
@@ -78,6 +79,34 @@ function serviceInternals(service: RateLimitService): { fetchAll: () => Promise<
return service as unknown as { fetchAll: () => Promise<void> }
}
type RateLimitWindow = Parameters<RateLimitService['attach']>[0]
class FakeRateLimitWindow extends EventEmitter {
webContents = {
send: vi.fn()
}
isDestroyed(): boolean {
return false
}
isVisible(): boolean {
return true
}
isMinimized(): boolean {
return false
}
isFocused(): boolean {
return true
}
}
function asRateLimitWindow(window: FakeRateLimitWindow): RateLimitWindow {
return window as unknown as RateLimitWindow
}
describe('RateLimitService', () => {
beforeEach(() => {
vi.clearAllMocks()
@@ -111,6 +140,36 @@ describe('RateLimitService', () => {
expect(fetchCodexRateLimits).toHaveBeenCalledTimes(2)
})
it('removes all window listeners when replacing the attached window', () => {
const service = new RateLimitService()
const firstWindow = new FakeRateLimitWindow()
const secondWindow = new FakeRateLimitWindow()
service.attach(asRateLimitWindow(firstWindow))
expect(firstWindow.listenerCount('focus')).toBe(1)
expect(firstWindow.listenerCount('show')).toBe(1)
expect(firstWindow.listenerCount('restore')).toBe(1)
expect(firstWindow.listenerCount('closed')).toBe(1)
service.attach(asRateLimitWindow(secondWindow))
expect(firstWindow.listenerCount('focus')).toBe(0)
expect(firstWindow.listenerCount('show')).toBe(0)
expect(firstWindow.listenerCount('restore')).toBe(0)
expect(firstWindow.listenerCount('closed')).toBe(0)
expect(secondWindow.listenerCount('focus')).toBe(1)
expect(secondWindow.listenerCount('show')).toBe(1)
expect(secondWindow.listenerCount('restore')).toBe(1)
expect(secondWindow.listenerCount('closed')).toBe(1)
service.stop()
expect(secondWindow.listenerCount('focus')).toBe(0)
expect(secondWindow.listenerCount('show')).toBe(0)
expect(secondWindow.listenerCount('restore')).toBe(0)
expect(secondWindow.listenerCount('closed')).toBe(0)
})
it('keeps recent stale data across repeated failures', async () => {
const service = new RateLimitService()
const internal = serviceInternals(service)
+15 -8
View File
@@ -153,21 +153,28 @@ export class RateLimitService {
const refreshOnResume = (): void => {
void this.refreshIfWindowActive()
}
mainWindow.on('focus', refreshOnResume)
mainWindow.on('show', refreshOnResume)
mainWindow.on('restore', refreshOnResume)
this.detachWindowListeners = () => {
// Why: attach() can replace windows; the previous closed listener also
// captures this service and must be removed with the focus listeners.
const detachWindowListeners = (): void => {
mainWindow.removeListener('focus', refreshOnResume)
mainWindow.removeListener('show', refreshOnResume)
mainWindow.removeListener('restore', refreshOnResume)
mainWindow.removeListener('closed', onClosed)
}
mainWindow.on('closed', () => {
this.detachWindowListeners?.()
this.detachWindowListeners = null
const onClosed = (): void => {
detachWindowListeners()
if (this.detachWindowListeners === detachWindowListeners) {
this.detachWindowListeners = null
}
if (this.mainWindow === mainWindow) {
this.mainWindow = null
}
})
}
mainWindow.on('focus', refreshOnResume)
mainWindow.on('show', refreshOnResume)
mainWindow.on('restore', refreshOnResume)
mainWindow.on('closed', onClosed)
this.detachWindowListeners = detachWindowListeners
}
start(): void {