Skip to content

Commit da0103f

Browse files
tiffanychumsteipete
tiffanychum
and
steipete
authored
fix(discord): messages sent during gateway reconnect are silently dropped (#100896)
* fix(discord): drain queued outbound deliveries after gateway reconnect (#56610) Discord sends that failed while the gateway websocket was reconnecting (close codes 1005/1006) stayed stuck in the outbound delivery queue and were silently dropped. Classify sends that fail while the registered gateway is disconnected as retryable, and drain pending discord queue entries when the gateway reconnects, matching the WhatsApp/Telegram reconnect drains. * fix(discord): harden reconnect recovery Co-authored-by: tiffanychum <[email protected]> * refactor(discord): remove unsafe reconnect replay * fix(discord): bound reconnect retry per request Co-authored-by: tiffanychum <[email protected]> * chore(changelog): leave reconnect fix to release flow --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent e8b0412 commit da0103f

10 files changed

Lines changed: 294 additions & 317 deletions

‎extensions/discord/src/client.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,39 @@
11
// Discord tests cover client plugin behavior.
22
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
33
import { afterEach, describe, expect, it, vi } from "vitest";
4-
import { createDiscordRestClient } from "./client.js";
4+
import { createDiscordClient, createDiscordRestClient } from "./client.js";
55
import type { RequestClient } from "./internal/discord.js";
6+
import type { GatewayPlugin } from "./internal/gateway.js";
7+
import { clearGateways, registerGateway } from "./monitor/gateway-registry.js";
68

79
afterEach(() => {
810
vi.unstubAllEnvs();
11+
clearGateways();
12+
});
13+
14+
describe("createDiscordClient", () => {
15+
it("extends a single REST operation after the registered gateway disconnects", async () => {
16+
registerGateway("default", { isConnected: false } as GatewayPlugin);
17+
const request = createDiscordClient({
18+
cfg: {
19+
channels: {
20+
discord: {
21+
token: "discord-token",
22+
retry: { attempts: 2, minDelayMs: 0, maxDelayMs: 0, jitter: 0 },
23+
},
24+
},
25+
},
26+
rest: {} as RequestClient,
27+
}).request;
28+
const operation = vi
29+
.fn()
30+
.mockRejectedValueOnce(new TypeError("fetch failed"))
31+
.mockRejectedValueOnce(new TypeError("fetch failed"))
32+
.mockResolvedValue("sent");
33+
34+
await expect(request(operation, "send")).resolves.toBe("sent");
35+
expect(operation).toHaveBeenCalledTimes(3);
36+
});
937
});
1038

1139
describe("createDiscordRestClient", () => {

‎extensions/discord/src/client.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
type ResolvedDiscordAccount,
1212
} from "./accounts.js";
1313
import { RequestClient } from "./internal/discord.js";
14+
import { getGateway } from "./monitor/gateway-registry.js";
1415
import { resolveDiscordProxyFetchForAccount } from "./proxy-fetch.js";
1516
import { createDiscordRequestClient } from "./proxy-request-client.js";
1617
import { createDiscordRetryRunner } from "./retry.js";
@@ -148,6 +149,10 @@ export function createDiscordClient(opts: DiscordClientOpts): {
148149
retry: opts.retry,
149150
configRetry: account.config.retry,
150151
verbose: opts.verbose,
152+
isGatewayDisconnected: () => {
153+
const gateway = getGateway(account.accountId);
154+
return gateway !== undefined && !gateway.isConnected;
155+
},
151156
});
152157
return { token, rest, request };
153158
}

‎extensions/discord/src/delivery-retry.ts‎

Lines changed: 0 additions & 56 deletions
This file was deleted.

‎extensions/discord/src/durable-delivery.test.ts‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,13 @@ describe("durable Discord delivery", () => {
4141
setActivePluginRegistry(createEmptyPluginRegistry());
4242
});
4343

44-
it("fans out planned text chunks and retries a transient failure on a later chunk", async () => {
44+
it("does not replay earlier chunks when a later platform send fails", async () => {
4545
hoisted.sendMessageDiscordMock
4646
.mockResolvedValueOnce({
4747
messageId: "msg-chunk-1",
4848
channelId: "ch-1",
4949
})
50-
.mockRejectedValueOnce(Object.assign(new Error("discord 500"), { status: 500 }))
51-
.mockResolvedValueOnce({
52-
messageId: "msg-chunk-2",
53-
channelId: "ch-1",
54-
});
50+
.mockRejectedValueOnce(Object.assign(new Error("discord 500"), { status: 500 }));
5551

5652
const result = await sendDurableMessageBatch({
5753
cfg: {
@@ -73,32 +69,22 @@ describe("durable Discord delivery", () => {
7369
skipQueue: true,
7470
});
7571

76-
expect(result.status).toBe("sent");
77-
if (result.status !== "sent") {
78-
throw new Error("expected durable Discord send to succeed");
72+
expect(result.status).toBe("partial_failed");
73+
if (result.status !== "partial_failed") {
74+
throw new Error("expected durable Discord send to report a partial failure");
7975
}
8076
expect(
8177
result.results.map((entry) => ({
8278
channel: entry.channel,
8379
messageId: entry.messageId,
8480
})),
85-
).toEqual([
86-
{ channel: "discord", messageId: "msg-chunk-1" },
87-
{ channel: "discord", messageId: "msg-chunk-2" },
88-
]);
89-
expect(result.receipt.platformMessageIds).toEqual(["msg-chunk-1", "msg-chunk-2"]);
90-
expect(result.payloadOutcomes).toEqual([
91-
{
92-
index: 0,
93-
status: "sent",
94-
results: result.results,
95-
},
96-
]);
97-
expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledTimes(3);
81+
).toEqual([{ channel: "discord", messageId: "msg-chunk-1" }]);
82+
expect(result.receipt.platformMessageIds).toEqual(["msg-chunk-1"]);
83+
expect(result.sentBeforeError).toBe(true);
84+
expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledTimes(2);
9885
expect(hoisted.sendMessageDiscordMock.mock.calls.map((call) => call[1])).toEqual([
9986
"first chunk",
10087
"second chunk",
101-
"second chunk",
10288
]);
10389
});
10490
});

‎extensions/discord/src/outbound-adapter.test.ts‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -162,35 +162,35 @@ describe("discordOutbound", () => {
162162
expect(options.chunkMode).toBe("newline");
163163
});
164164

165-
it.each([500, 429])("retries transient Discord text send status %i", async (status) => {
166-
hoisted.sendMessageDiscordMock
167-
.mockRejectedValueOnce(Object.assign(new Error(`discord ${status}`), { status }))
168-
.mockResolvedValueOnce({
169-
messageId: "msg-retry-ok",
170-
channelId: "ch-1",
171-
});
172-
173-
const result = await discordOutbound.sendText?.({
174-
cfg: {
175-
channels: {
176-
discord: {
177-
token: "test-token",
178-
retry: { attempts: 2, minDelayMs: 0, maxDelayMs: 0, jitter: 0 },
165+
it.each([500, 429])(
166+
"does not replay an injected Discord delivery after status %i",
167+
async (status) => {
168+
hoisted.sendMessageDiscordMock
169+
.mockRejectedValueOnce(Object.assign(new Error(`discord ${status}`), { status }))
170+
.mockResolvedValueOnce({
171+
messageId: "msg-retry-ok",
172+
channelId: "ch-1",
173+
});
174+
175+
await expect(
176+
discordOutbound.sendText?.({
177+
cfg: {
178+
channels: {
179+
discord: {
180+
token: "test-token",
181+
retry: { attempts: 2, minDelayMs: 0, maxDelayMs: 0, jitter: 0 },
182+
},
183+
},
179184
},
180-
},
181-
},
182-
to: "channel:123456",
183-
text: "retry me",
184-
accountId: "default",
185-
});
185+
to: "channel:123456",
186+
text: "do not replay me",
187+
accountId: "default",
188+
}),
189+
).rejects.toThrow(`discord ${status}`);
186190

187-
expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledTimes(2);
188-
expect(result).toEqual({
189-
channel: "discord",
190-
messageId: "msg-retry-ok",
191-
channelId: "ch-1",
192-
});
193-
});
191+
expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledTimes(1);
192+
},
193+
);
194194

195195
it("uses webhook persona delivery for bound thread text replies", async () => {
196196
mockDiscordBoundThreadManager(hoisted);

0 commit comments

Comments
 (0)