Skip to content

Bug: singleton getInstance(client) silently discards client on subsequent calls #458

@lonix

Description

@lonix

Problem

Multiple services use a singleton pattern that accepts a required client: Client parameter in getInstance() but silently ignores it after the first call:

// e.g. src/services/voice-channel-manager.ts
public static getInstance(client: Client): VoiceChannelManager {
  if (!VoiceChannelManager.instance) {
    VoiceChannelManager.instance = new VoiceChannelManager(client);
  }
  return VoiceChannelManager.instance; // ← client arg silently ignored after first call
}

The same pattern appears in at least:

  • VoiceChannelManager
  • PermissionsService
  • PollService
  • NoticesChannelManager
  • ScheduledAnnouncementService
  • VoiceChannelAnnouncer
  • LeaderboardRoleService
  • CommandManager

Why this is a problem

  1. Misleading API: callers believe they are injecting a dependency, but the second call silently returns an instance bound to a completely different client. In tests this leads to subtle state pollution between test cases.
  2. Test isolation: Jest test files that call getInstance(mockClient) after another test already populated the static field will receive the previous test's client, causing hard-to-diagnose failures.
  3. Reconnect scenarios: if the Discord Client is ever recreated (e.g. after a fatal gateway disconnect), passing the new client to getInstance() will have no effect.

Suggested fix

Option A — make client a separate setClient() call (least-disruptive):

public static getInstance(): VoiceChannelManager {
  if (!VoiceChannelManager.instance) {
    VoiceChannelManager.instance = new VoiceChannelManager();
  }
  return VoiceChannelManager.instance;
}

public setClient(client: Client): void {
  this.client = client;
}

Call setClient() once at startup, before any other method.

Option B — assert client consistency (defensive):

public static getInstance(client: Client): VoiceChannelManager {
  if (!VoiceChannelManager.instance) {
    VoiceChannelManager.instance = new VoiceChannelManager(client);
  } else if (VoiceChannelManager.instance.client !== client) {
    throw new Error("VoiceChannelManager already initialised with a different client");
  }
  return VoiceChannelManager.instance;
}

Option C — reset static field in tests (minimal change, test-only fix):
Add a static reset() method used only in test setup/teardown to clear the static instance.

Option B or C is the quickest win; Option A is the cleanest long-term design.

Affected files

src/services/voice-channel-manager.ts, src/services/permissions-service.ts, src/services/poll-service.ts, src/services/notices-channel-manager.ts, src/services/scheduled-announcement-service.ts, src/services/voice-channel-announcer.ts, src/services/leaderboard-role-service.ts, src/services/command-manager.ts

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions