mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 13:50:49 +00:00
fix(discord): clear stale heartbeat timers in SafeGatewayPlugin.connect() (#65087)
* fix(discord): clear stale heartbeat timers in SafeGatewayPlugin.connect() The @buape/carbon@0.15.0 heartbeat setup has a race where stopHeartbeat() runs before heartbeatInterval is assigned, leaving a stale setInterval with a closed reconnectCallback. When the stale interval fires ~41s later it throws an uncaught exception that bypasses the EventEmitter error path and crashes the gateway process via process.on('uncaughtException'). Add a connect() override in SafeGatewayPlugin that unconditionally clears both heartbeatInterval and firstHeartbeatTimeout before calling super. The parent's connect() only calls stopHeartbeat() when isConnecting=false; when isConnecting=true it returns early without clearing — this override fills that gap. Fixes #65009. Related: #64011, #63387, #62038. * test(discord): assert super.connect() delegation in SafeGatewayPlugin tests * fix(ci): update raw-fetch allowlist line numbers for gateway-plugin.ts The connect() override added in the heartbeat fix shifted the two pre-existing fetch() callsites from lines 370/436 to 387/453. * docs(changelog): add discord heartbeat crash note * test(cli): align plugin registry load-context mock --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai
|
||||
- QA/packaging: stop packaged QA helpers from crashing when optional scenario execution config is unavailable, so npm distributions can skip the repo-only scenario pack without breaking completion-cache and startup paths. (#65118) Thanks @EdderTalmor and @vincentkoc.
|
||||
- Media/audio transcription: surface the real provider failure when every audio transcription attempt fails, so status output and the CLI stop collapsing those errors into generic skips. (#65096) Thanks @l0cka and @vincentkoc.
|
||||
- Memory/wiki: preserve Unicode letters, digits, and combining marks in wiki slugs and contradiction clustering, and cap Unicode filename segments to safe byte lengths so non-ASCII titles stop collapsing or overflowing path limits. (#64742) Thanks @zhouhe-xydt and @vincentkoc.
|
||||
- Discord/gateway: clear stale heartbeat timers before reconnecting so zombie gateway callbacks cannot crash the process and drop in-flight replies. (#65009) Thanks @SARAMALI15792 and @vincentkoc.
|
||||
|
||||
## 2026.4.12
|
||||
|
||||
|
||||
115
extensions/discord/src/monitor/gateway-plugin.test.ts
Normal file
115
extensions/discord/src/monitor/gateway-plugin.test.ts
Normal file
@@ -0,0 +1,115 @@
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const { baseConnectSpy, GatewayIntents, GatewayPlugin } = vi.hoisted(() => {
|
||||
const baseConnectSpy = vi.fn<(resume: boolean) => void>();
|
||||
|
||||
const GatewayIntents = {
|
||||
Guilds: 1 << 0,
|
||||
GuildMessages: 1 << 1,
|
||||
MessageContent: 1 << 2,
|
||||
DirectMessages: 1 << 3,
|
||||
GuildMessageReactions: 1 << 4,
|
||||
DirectMessageReactions: 1 << 5,
|
||||
GuildPresences: 1 << 6,
|
||||
GuildMembers: 1 << 7,
|
||||
} as const;
|
||||
|
||||
class GatewayPlugin {
|
||||
options: unknown;
|
||||
gatewayInfo: unknown;
|
||||
heartbeatInterval: ReturnType<typeof setInterval> | undefined = undefined;
|
||||
firstHeartbeatTimeout: ReturnType<typeof setTimeout> | undefined = undefined;
|
||||
isConnecting: boolean = false;
|
||||
|
||||
constructor(options?: unknown) {
|
||||
this.options = options;
|
||||
}
|
||||
|
||||
async registerClient(_client: unknown): Promise<void> {}
|
||||
|
||||
connect(resume = false): void {
|
||||
baseConnectSpy(resume);
|
||||
}
|
||||
}
|
||||
|
||||
return { baseConnectSpy, GatewayIntents, GatewayPlugin };
|
||||
});
|
||||
|
||||
vi.mock("@buape/carbon/gateway", () => ({ GatewayIntents, GatewayPlugin }));
|
||||
|
||||
vi.mock("@buape/carbon/dist/src/plugins/gateway/index.js", () => ({
|
||||
GatewayIntents,
|
||||
GatewayPlugin,
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/proxy-capture", () => ({
|
||||
captureHttpExchange: vi.fn(),
|
||||
captureWsEvent: vi.fn(),
|
||||
resolveEffectiveDebugProxyUrl: () => undefined,
|
||||
resolveDebugProxySettings: () => ({ enabled: false }),
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/runtime-env", () => ({
|
||||
danger: (value: string) => value,
|
||||
}));
|
||||
|
||||
describe("SafeGatewayPlugin.connect()", () => {
|
||||
let createDiscordGatewayPlugin: typeof import("./gateway-plugin.js").createDiscordGatewayPlugin;
|
||||
|
||||
beforeAll(async () => {
|
||||
({ createDiscordGatewayPlugin } = await import("./gateway-plugin.js"));
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
baseConnectSpy.mockClear();
|
||||
});
|
||||
|
||||
function createPlugin() {
|
||||
return createDiscordGatewayPlugin({
|
||||
discordConfig: {},
|
||||
runtime: {
|
||||
log: vi.fn(),
|
||||
error: vi.fn(),
|
||||
exit: vi.fn(),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
it("clears stale heartbeatInterval before delegating to super when isConnecting=true", () => {
|
||||
const plugin = createPlugin();
|
||||
|
||||
const staleInterval = setInterval(() => {}, 99_999);
|
||||
try {
|
||||
plugin.heartbeatInterval = staleInterval;
|
||||
|
||||
// isConnecting is private on GatewayPlugin — cast required.
|
||||
(plugin as unknown as { isConnecting: boolean }).isConnecting = true;
|
||||
|
||||
plugin.connect(false);
|
||||
|
||||
expect(plugin.heartbeatInterval).toBeUndefined();
|
||||
expect(baseConnectSpy).toHaveBeenCalledWith(false);
|
||||
} finally {
|
||||
clearInterval(staleInterval);
|
||||
}
|
||||
});
|
||||
|
||||
it("clears stale firstHeartbeatTimeout before delegating to super when isConnecting=true", () => {
|
||||
const plugin = createPlugin();
|
||||
|
||||
const staleTimeout = setTimeout(() => {}, 99_999);
|
||||
try {
|
||||
plugin.firstHeartbeatTimeout = staleTimeout;
|
||||
|
||||
// isConnecting is private on GatewayPlugin — cast required.
|
||||
(plugin as unknown as { isConnecting: boolean }).isConnecting = true;
|
||||
|
||||
plugin.connect(false);
|
||||
|
||||
expect(plugin.firstHeartbeatTimeout).toBeUndefined();
|
||||
expect(baseConnectSpy).toHaveBeenCalledWith(false);
|
||||
} finally {
|
||||
clearTimeout(staleTimeout);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -254,6 +254,23 @@ function createGatewayPlugin(params: {
|
||||
super(params.options);
|
||||
}
|
||||
|
||||
public override connect(resume = false): void {
|
||||
// Guard against stale heartbeat timers from the @buape/carbon
|
||||
// firstHeartbeatTimeout race (openclaw/openclaw#65009, #64011, #63387).
|
||||
// Parent connect() only calls stopHeartbeat() when isConnecting=false.
|
||||
// If isConnecting=true it returns early — leaving a stale setInterval
|
||||
// that fires with a closed reconnectCallback and crashes the process.
|
||||
if (this.heartbeatInterval !== undefined) {
|
||||
clearInterval(this.heartbeatInterval);
|
||||
this.heartbeatInterval = undefined;
|
||||
}
|
||||
if (this.firstHeartbeatTimeout !== undefined) {
|
||||
clearTimeout(this.firstHeartbeatTimeout);
|
||||
this.firstHeartbeatTimeout = undefined;
|
||||
}
|
||||
super.connect(resume);
|
||||
}
|
||||
|
||||
override async registerClient(
|
||||
client: Parameters<carbonGateway.GatewayPlugin["registerClient"]>[0],
|
||||
) {
|
||||
|
||||
@@ -22,8 +22,8 @@ const allowedRawFetchCallsites = new Set([
|
||||
bundledPluginCallsite("browser", "src/browser/test-fetch.ts", 27),
|
||||
bundledPluginCallsite("chutes", "models.ts", 535),
|
||||
bundledPluginCallsite("chutes", "models.ts", 542),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 370),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 436),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 387),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 453),
|
||||
bundledPluginCallsite("discord", "src/voice-message.ts", 298),
|
||||
bundledPluginCallsite("discord", "src/voice-message.ts", 333),
|
||||
bundledPluginCallsite("elevenlabs", "speech-provider.ts", 295),
|
||||
|
||||
@@ -8,6 +8,25 @@ const logger = {
|
||||
debug: vi.fn(),
|
||||
};
|
||||
|
||||
function withActivatedPluginIdsForTest<T extends Record<string, unknown>>(
|
||||
config: T,
|
||||
pluginIds: string[],
|
||||
): T & {
|
||||
plugins: {
|
||||
allow: string[];
|
||||
entries: Record<string, { enabled: true }>;
|
||||
};
|
||||
} {
|
||||
return {
|
||||
...config,
|
||||
plugins: {
|
||||
...(typeof config.plugins === "object" && config.plugins ? config.plugins : {}),
|
||||
allow: pluginIds,
|
||||
entries: Object.fromEntries(pluginIds.map((pluginId) => [pluginId, { enabled: true }])),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
loadOpenClawPlugins: vi.fn<typeof import("../plugins/loader.js").loadOpenClawPlugins>(),
|
||||
getActivePluginRegistry: vi.fn<typeof import("../plugins/runtime.js").getActivePluginRegistry>(),
|
||||
@@ -44,6 +63,25 @@ vi.mock("../plugins/runtime/load-context.js", () => ({
|
||||
resolvePluginRuntimeLoadContext: (
|
||||
...args: Parameters<typeof mocks.resolvePluginRuntimeLoadContext>
|
||||
) => mocks.resolvePluginRuntimeLoadContext(...args),
|
||||
buildPluginRuntimeLoadOptionsFromValues: (
|
||||
values: {
|
||||
config: unknown;
|
||||
activationSourceConfig: unknown;
|
||||
autoEnabledReasons: Readonly<Record<string, string[]>>;
|
||||
workspaceDir: string | undefined;
|
||||
env: NodeJS.ProcessEnv;
|
||||
logger: typeof logger;
|
||||
},
|
||||
overrides?: Record<string, unknown>,
|
||||
) => ({
|
||||
config: values.config,
|
||||
activationSourceConfig: values.activationSourceConfig,
|
||||
autoEnabledReasons: values.autoEnabledReasons,
|
||||
workspaceDir: values.workspaceDir,
|
||||
env: values.env,
|
||||
logger: values.logger,
|
||||
...overrides,
|
||||
}),
|
||||
buildPluginRuntimeLoadOptions: (
|
||||
context: {
|
||||
config: unknown;
|
||||
@@ -107,16 +145,7 @@ describe("ensurePluginRegistryLoaded", () => {
|
||||
},
|
||||
},
|
||||
};
|
||||
const autoEnabledConfig = {
|
||||
...baseConfig,
|
||||
plugins: {
|
||||
entries: {
|
||||
"demo-chat": {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
const autoEnabledConfig = withActivatedPluginIdsForTest(baseConfig, ["demo-chat"]);
|
||||
|
||||
mocks.resolvePluginRuntimeLoadContext.mockReturnValue({
|
||||
rawConfig: baseConfig,
|
||||
@@ -143,7 +172,7 @@ describe("ensurePluginRegistryLoaded", () => {
|
||||
expect(mocks.loadOpenClawPlugins).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: autoEnabledConfig,
|
||||
activationSourceConfig: baseConfig,
|
||||
activationSourceConfig: autoEnabledConfig,
|
||||
autoEnabledReasons: {
|
||||
"demo-chat": ["demo-chat configured"],
|
||||
},
|
||||
@@ -230,6 +259,7 @@ describe("ensurePluginRegistryLoaded", () => {
|
||||
plugins: { enabled: true },
|
||||
channels: { "demo-channel-a": { enabled: true } },
|
||||
};
|
||||
const activatedConfig = withActivatedPluginIdsForTest(config, ["demo-channel-a"]);
|
||||
|
||||
mocks.resolvePluginRuntimeLoadContext.mockReturnValue({
|
||||
rawConfig: config,
|
||||
@@ -252,7 +282,8 @@ describe("ensurePluginRegistryLoaded", () => {
|
||||
expect(mocks.loadOpenClawPlugins).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.loadOpenClawPlugins).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config,
|
||||
config: activatedConfig,
|
||||
activationSourceConfig: activatedConfig,
|
||||
onlyPluginIds: ["demo-channel-a"],
|
||||
throwOnLoadError: true,
|
||||
workspaceDir: "/tmp/workspace",
|
||||
@@ -270,6 +301,7 @@ describe("ensurePluginRegistryLoaded", () => {
|
||||
},
|
||||
},
|
||||
};
|
||||
const activatedConfig = withActivatedPluginIdsForTest(config, ["demo-channel-a"]);
|
||||
|
||||
mocks.resolvePluginRuntimeLoadContext.mockReturnValue({
|
||||
rawConfig: config,
|
||||
@@ -291,7 +323,8 @@ describe("ensurePluginRegistryLoaded", () => {
|
||||
expect(mocks.loadOpenClawPlugins).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.loadOpenClawPlugins).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config,
|
||||
config: activatedConfig,
|
||||
activationSourceConfig: activatedConfig,
|
||||
onlyPluginIds: ["demo-channel-a"],
|
||||
throwOnLoadError: true,
|
||||
workspaceDir: "/tmp/workspace",
|
||||
|
||||
Reference in New Issue
Block a user