mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:50:44 +00:00
fix(discord): prevent identify race (#68159)
Verified against Carbon 0.16.0 source:
- Client constructor calls plugin.registerClient(this) without awaiting it.
- GatewayPlugin.registerClient publishes client before its awaited metadata fetch.
- identify() silently returns when client is missing.
This patch matches Carbon's ordering in OpenClaw's subclass, avoids a second super.registerClient call if lifecycle connect already opened the socket during metadata loading, and keeps regression coverage for both ws and isConnecting cases.
Local proof:
- pnpm test extensions/discord/src/monitor/provider.proxy.test.ts extensions/discord/src/monitor/gateway-plugin.test.ts
- pnpm lint:tmp:no-raw-channel-fetch
- pnpm check:changed
- pnpm check
- pnpm test
GitHub checks green for 72547825e1.
This commit is contained in:
@@ -57,6 +57,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Discord/gateway: prevent startup from getting stuck at `awaiting gateway readiness` when Carbon gateway registration races with a lifecycle reconnect. Fixes #52372. (#68159) Thanks @IVY-AI-gif.
|
||||
- Plugin SDK/tool-result transforms: restrict harness tool-result middleware to bundled plugins, fail closed on middleware errors, validate rewritten result shapes, preserve Pi per-call ids, and keep Codex media trust checks anchored to raw tool provenance. Thanks @vincentkoc.
|
||||
- Plugins/Google Chat: log webhook auth rejection reasons only after all candidates fail, and warn when add-on `appPrincipal` values do not match configuration. Fixes #71078. (#71145) Thanks @luyao618.
|
||||
- Models/configure: preserve the existing default model when provider auth is re-run from configure while keeping explicit default-setting commands authoritative. Fixes #70696. (#70793) Thanks @Sathvik-1007.
|
||||
|
||||
@@ -32,6 +32,23 @@ type DiscordGatewayFetch = (
|
||||
|
||||
type DiscordGatewayMetadataError = Error & { transient?: boolean };
|
||||
type DiscordGatewayWebSocketCtor = new (url: string, options?: { agent?: unknown }) => ws.WebSocket;
|
||||
type CarbonGatewayRegistrationState = {
|
||||
client?: Parameters<carbonGateway.GatewayPlugin["registerClient"]>[0];
|
||||
ws?: unknown;
|
||||
isConnecting?: boolean;
|
||||
};
|
||||
|
||||
function assignCarbonGatewayClient(
|
||||
plugin: carbonGateway.GatewayPlugin,
|
||||
client: Parameters<carbonGateway.GatewayPlugin["registerClient"]>[0],
|
||||
): void {
|
||||
(plugin as unknown as CarbonGatewayRegistrationState).client = client;
|
||||
}
|
||||
|
||||
function hasCarbonGatewaySocketStarted(plugin: carbonGateway.GatewayPlugin): boolean {
|
||||
const state = plugin as unknown as CarbonGatewayRegistrationState;
|
||||
return state.ws != null || state.isConnecting === true;
|
||||
}
|
||||
|
||||
export function resolveDiscordGatewayIntents(
|
||||
intentsConfig?: import("openclaw/plugin-sdk/config-runtime").DiscordIntentsConfig,
|
||||
@@ -274,6 +291,12 @@ function createGatewayPlugin(params: {
|
||||
override async registerClient(
|
||||
client: Parameters<carbonGateway.GatewayPlugin["registerClient"]>[0],
|
||||
) {
|
||||
// Carbon's Client constructor does not await plugin registerClient().
|
||||
// Match Carbon's own GatewayPlugin ordering by publishing the client
|
||||
// reference before our metadata fetch can yield, so an external
|
||||
// connect()->identify() cannot silently drop IDENTIFY (#52372).
|
||||
assignCarbonGatewayClient(this, client);
|
||||
|
||||
if (!this.gatewayInfo || this.gatewayInfoUsedFallback) {
|
||||
const resolved = await fetchDiscordGatewayInfoWithTimeout({
|
||||
token: client.options.token,
|
||||
@@ -292,6 +315,13 @@ function createGatewayPlugin(params: {
|
||||
await params.testing.registerClient(this, client);
|
||||
return;
|
||||
}
|
||||
// If the lifecycle timeout already started a socket while metadata was
|
||||
// loading, do not call Carbon's registerClient() again; it would close
|
||||
// that socket and open another one. Carbon stores these as runtime fields
|
||||
// even though they are protected/private in the .d.ts.
|
||||
if (hasCarbonGatewaySocketStarted(this)) {
|
||||
return;
|
||||
}
|
||||
return super.registerClient(client);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
function resolveGatewayInfoFetch(resolve: ((value: Response) => void) | undefined): void {
|
||||
expect(resolve).toBeDefined();
|
||||
resolve!({
|
||||
ok: true,
|
||||
status: 200,
|
||||
text: async () => JSON.stringify({ url: "wss://gateway.discord.gg" }),
|
||||
} as Response);
|
||||
}
|
||||
|
||||
const {
|
||||
GatewayIntents,
|
||||
baseRegisterClientSpy,
|
||||
@@ -42,9 +51,15 @@ const {
|
||||
class GatewayPlugin {
|
||||
options: unknown;
|
||||
gatewayInfo: unknown;
|
||||
client: unknown;
|
||||
ws: unknown;
|
||||
isConnecting: boolean;
|
||||
constructor(options?: unknown, gatewayInfo?: unknown) {
|
||||
this.options = options;
|
||||
this.gatewayInfo = gatewayInfo;
|
||||
this.client = undefined;
|
||||
this.ws = undefined;
|
||||
this.isConnecting = false;
|
||||
}
|
||||
async registerClient(client: unknown) {
|
||||
baseRegisterClientSpy(client);
|
||||
@@ -445,6 +460,89 @@ describe("createDiscordGatewayPlugin", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("sets client reference before the async gateway-info fetch resolves (regression for #52372)", async () => {
|
||||
vi.useFakeTimers();
|
||||
const runtime = createRuntime();
|
||||
let fetchResolve: ((v: Response) => void) | undefined;
|
||||
globalFetchMock.mockImplementation(
|
||||
() =>
|
||||
new Promise<Response>((resolve) => {
|
||||
fetchResolve = resolve;
|
||||
}),
|
||||
);
|
||||
const plugin = createDiscordGatewayPlugin({
|
||||
discordConfig: {},
|
||||
runtime,
|
||||
});
|
||||
|
||||
const clientArg = {
|
||||
options: { token: "token-race" },
|
||||
registerListener: baseRegisterClientSpy,
|
||||
unregisterListener: vi.fn(),
|
||||
};
|
||||
const registerPromise = (
|
||||
plugin as unknown as {
|
||||
registerClient: (c: typeof clientArg) => Promise<void>;
|
||||
}
|
||||
).registerClient(clientArg);
|
||||
|
||||
// Before the metadata fetch resolves, this.client should already be set so
|
||||
// that a concurrent identify() cannot observe an undefined client.
|
||||
expect((plugin as unknown as { client: unknown }).client).toBe(clientArg);
|
||||
|
||||
resolveGatewayInfoFetch(fetchResolve);
|
||||
await registerPromise;
|
||||
|
||||
expect(baseRegisterClientSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("skips super.registerClient when an external connect starts during the metadata fetch (regression for #52372)", async () => {
|
||||
const runCase = async (markStarted: (plugin: unknown) => void) => {
|
||||
vi.useFakeTimers();
|
||||
const runtime = createRuntime();
|
||||
let fetchResolve: ((v: Response) => void) | undefined;
|
||||
globalFetchMock.mockImplementation(
|
||||
() =>
|
||||
new Promise<Response>((resolve) => {
|
||||
fetchResolve = resolve;
|
||||
}),
|
||||
);
|
||||
const plugin = createDiscordGatewayPlugin({
|
||||
discordConfig: {},
|
||||
runtime,
|
||||
});
|
||||
|
||||
const clientArg = {
|
||||
options: { token: "token-race" },
|
||||
registerListener: baseRegisterClientSpy,
|
||||
unregisterListener: vi.fn(),
|
||||
};
|
||||
const registerPromise = (
|
||||
plugin as unknown as {
|
||||
registerClient: (c: typeof clientArg) => Promise<void>;
|
||||
}
|
||||
).registerClient(clientArg);
|
||||
|
||||
markStarted(plugin);
|
||||
resolveGatewayInfoFetch(fetchResolve);
|
||||
await registerPromise;
|
||||
};
|
||||
|
||||
await runCase((plugin) => {
|
||||
(plugin as { ws: unknown }).ws = { readyState: 1 };
|
||||
});
|
||||
expect(baseRegisterClientSpy).not.toHaveBeenCalled();
|
||||
|
||||
baseRegisterClientSpy.mockClear();
|
||||
globalFetchMock.mockReset();
|
||||
vi.useRealTimers();
|
||||
|
||||
await runCase((plugin) => {
|
||||
(plugin as { isConnecting: boolean }).isConnecting = true;
|
||||
});
|
||||
expect(baseRegisterClientSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refreshes fallback gateway metadata on the next register attempt", async () => {
|
||||
const runtime = createRuntime();
|
||||
globalFetchMock
|
||||
|
||||
@@ -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", 387),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 453),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 417),
|
||||
bundledPluginCallsite("discord", "src/monitor/gateway-plugin.ts", 483),
|
||||
bundledPluginCallsite("discord", "src/voice-message.ts", 298),
|
||||
bundledPluginCallsite("discord", "src/voice-message.ts", 333),
|
||||
bundledPluginCallsite("elevenlabs", "speech-provider.ts", 295),
|
||||
|
||||
Reference in New Issue
Block a user