mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 16:30:57 +00:00
fix(discord): surface final reply permission context
This commit is contained in:
@@ -98,6 +98,21 @@ function isProcessAborted(abortSignal?: AbortSignal): boolean {
|
|||||||
return Boolean(abortSignal?.aborted);
|
return Boolean(abortSignal?.aborted);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function formatDiscordReplyDeliveryFailure(params: {
|
||||||
|
kind: string;
|
||||||
|
err: unknown;
|
||||||
|
target: string;
|
||||||
|
sessionKey?: string;
|
||||||
|
}) {
|
||||||
|
const context = [
|
||||||
|
`target=${params.target}`,
|
||||||
|
params.sessionKey ? `session=${params.sessionKey}` : undefined,
|
||||||
|
]
|
||||||
|
.filter(Boolean)
|
||||||
|
.join(" ");
|
||||||
|
return `discord ${params.kind} reply failed (${context}): ${String(params.err)}`;
|
||||||
|
}
|
||||||
|
|
||||||
type DiscordMessageProcessObserver = {
|
type DiscordMessageProcessObserver = {
|
||||||
onFinalReplyStart?: () => void;
|
onFinalReplyStart?: () => void;
|
||||||
onFinalReplyDelivered?: () => void;
|
onFinalReplyDelivered?: () => void;
|
||||||
@@ -889,7 +904,16 @@ export async function processDiscordMessage(
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
onError: (err, info) => {
|
onError: (err, info) => {
|
||||||
runtime.error?.(danger(`discord ${info.kind} reply failed: ${String(err)}`));
|
runtime.error?.(
|
||||||
|
danger(
|
||||||
|
formatDiscordReplyDeliveryFailure({
|
||||||
|
kind: info.kind,
|
||||||
|
err,
|
||||||
|
target: deliverTarget,
|
||||||
|
sessionKey: ctxPayload.SessionKey,
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
);
|
||||||
},
|
},
|
||||||
onReplyStart: async () => {
|
onReplyStart: async () => {
|
||||||
if (isProcessAborted(abortSignal)) {
|
if (isProcessAborted(abortSignal)) {
|
||||||
|
|||||||
@@ -10,6 +10,9 @@ const sendMessageDiscordMock = vi.hoisted(() => vi.fn());
|
|||||||
const sendVoiceMessageDiscordMock = vi.hoisted(() => vi.fn());
|
const sendVoiceMessageDiscordMock = vi.hoisted(() => vi.fn());
|
||||||
const sendWebhookMessageDiscordMock = vi.hoisted(() => vi.fn());
|
const sendWebhookMessageDiscordMock = vi.hoisted(() => vi.fn());
|
||||||
const sendDiscordTextMock = vi.hoisted(() => vi.fn());
|
const sendDiscordTextMock = vi.hoisted(() => vi.fn());
|
||||||
|
const buildDiscordSendErrorMock = vi.hoisted(() =>
|
||||||
|
vi.fn<(err: unknown, ctx?: unknown) => Promise<unknown>>(async (err: unknown) => err),
|
||||||
|
);
|
||||||
const retryAsyncMock = vi.hoisted(() =>
|
const retryAsyncMock = vi.hoisted(() =>
|
||||||
vi.fn(
|
vi.fn(
|
||||||
async (
|
async (
|
||||||
@@ -47,6 +50,7 @@ vi.mock("../send.js", async () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
vi.mock("../send.shared.js", () => ({
|
vi.mock("../send.shared.js", () => ({
|
||||||
|
buildDiscordSendError: (err: unknown, ctx: unknown) => buildDiscordSendErrorMock(err, ctx),
|
||||||
sendDiscordText: (...args: unknown[]) => sendDiscordTextMock(...args),
|
sendDiscordText: (...args: unknown[]) => sendDiscordTextMock(...args),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
@@ -135,6 +139,7 @@ describe("deliverDiscordReply", () => {
|
|||||||
id: "msg-direct-1",
|
id: "msg-direct-1",
|
||||||
channel_id: "channel-1",
|
channel_id: "channel-1",
|
||||||
});
|
});
|
||||||
|
buildDiscordSendErrorMock.mockClear().mockImplementation(async (err: unknown) => err);
|
||||||
retryAsyncMock.mockClear();
|
retryAsyncMock.mockClear();
|
||||||
threadBindingTesting.resetThreadBindingsForTests();
|
threadBindingTesting.resetThreadBindingsForTests();
|
||||||
});
|
});
|
||||||
@@ -485,6 +490,40 @@ describe("deliverDiscordReply", () => {
|
|||||||
expect(sendMessageDiscordMock).toHaveBeenCalledTimes(1);
|
expect(sendMessageDiscordMock).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("wraps direct REST permission errors with channel context", async () => {
|
||||||
|
const apiErr = Object.assign(new Error("Missing Permissions"), {
|
||||||
|
code: 50013,
|
||||||
|
status: 403,
|
||||||
|
});
|
||||||
|
const wrappedErr = new Error(
|
||||||
|
"discord missing permissions in channel 789; permission probe did not identify missing ViewChannel/SendMessages (code=50013 status=403)",
|
||||||
|
);
|
||||||
|
sendDiscordTextMock.mockRejectedValueOnce(apiErr);
|
||||||
|
buildDiscordSendErrorMock.mockResolvedValueOnce(wrappedErr);
|
||||||
|
|
||||||
|
const fakeRest = {
|
||||||
|
post: vi.fn(),
|
||||||
|
get: vi.fn(),
|
||||||
|
} as unknown as import("@buape/carbon").RequestClient;
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
deliverDiscordReply({
|
||||||
|
replies: [{ text: "fail" }],
|
||||||
|
target: "channel:789",
|
||||||
|
token: "token",
|
||||||
|
rest: fakeRest,
|
||||||
|
runtime,
|
||||||
|
cfg,
|
||||||
|
textLimit: 2000,
|
||||||
|
}),
|
||||||
|
).rejects.toThrow("discord missing permissions in channel 789");
|
||||||
|
|
||||||
|
expect(buildDiscordSendErrorMock).toHaveBeenCalledWith(
|
||||||
|
apiErr,
|
||||||
|
expect.objectContaining({ channelId: "789", hasMedia: false }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("throws after exhausting retry attempts", async () => {
|
it("throws after exhausting retry attempts", async () => {
|
||||||
const rateLimitErr = Object.assign(new Error("rate limited"), { status: 429 });
|
const rateLimitErr = Object.assign(new Error("rate limited"), { status: 429 });
|
||||||
sendMessageDiscordMock.mockRejectedValue(rateLimitErr);
|
sendMessageDiscordMock.mockRejectedValue(rateLimitErr);
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ import { chunkDiscordTextWithMode } from "../chunk.js";
|
|||||||
import { isLikelyDiscordVideoMedia } from "../media-detection.js";
|
import { isLikelyDiscordVideoMedia } from "../media-detection.js";
|
||||||
import { createDiscordRetryRunner } from "../retry.js";
|
import { createDiscordRetryRunner } from "../retry.js";
|
||||||
import { sendMessageDiscord, sendVoiceMessageDiscord, sendWebhookMessageDiscord } from "../send.js";
|
import { sendMessageDiscord, sendVoiceMessageDiscord, sendWebhookMessageDiscord } from "../send.js";
|
||||||
import { sendDiscordText } from "../send.shared.js";
|
import { buildDiscordSendError, sendDiscordText } from "../send.shared.js";
|
||||||
|
|
||||||
export type DiscordThreadBindingLookupRecord = {
|
export type DiscordThreadBindingLookupRecord = {
|
||||||
accountId: string;
|
accountId: string;
|
||||||
@@ -322,21 +322,31 @@ async function sendDiscordChunkWithFallback(params: {
|
|||||||
// that can cause ordering issues under queue contention or rate limiting.
|
// that can cause ordering issues under queue contention or rate limiting.
|
||||||
if (params.channelId && params.request && params.rest) {
|
if (params.channelId && params.request && params.rest) {
|
||||||
const { channelId, request, rest } = params;
|
const { channelId, request, rest } = params;
|
||||||
await sendWithRetry(
|
try {
|
||||||
() =>
|
await sendWithRetry(
|
||||||
sendDiscordText(
|
() =>
|
||||||
rest,
|
sendDiscordText(
|
||||||
channelId,
|
rest,
|
||||||
text,
|
channelId,
|
||||||
params.replyTo,
|
text,
|
||||||
request,
|
params.replyTo,
|
||||||
params.maxLinesPerMessage,
|
request,
|
||||||
undefined,
|
params.maxLinesPerMessage,
|
||||||
undefined,
|
undefined,
|
||||||
params.chunkMode,
|
undefined,
|
||||||
),
|
params.chunkMode,
|
||||||
params.retryConfig,
|
),
|
||||||
);
|
params.retryConfig,
|
||||||
|
);
|
||||||
|
} catch (err) {
|
||||||
|
throw await buildDiscordSendError(err, {
|
||||||
|
channelId,
|
||||||
|
cfg: params.cfg,
|
||||||
|
rest,
|
||||||
|
token: params.token,
|
||||||
|
hasMedia: false,
|
||||||
|
});
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
await sendWithRetry(
|
await sendWithRetry(
|
||||||
|
|||||||
@@ -355,6 +355,41 @@ describe("sendMessageDiscord", () => {
|
|||||||
expect(String(error)).toMatch(/SendMessages/);
|
expect(String(error)).toMatch(/SendMessages/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("keeps 50013 context when permission probe finds baseline permissions", async () => {
|
||||||
|
const { rest, postMock, getMock } = makeDiscordRest();
|
||||||
|
const perms = PermissionFlagsBits.ViewChannel | PermissionFlagsBits.SendMessages;
|
||||||
|
const apiError = Object.assign(new Error("Missing Permissions"), {
|
||||||
|
code: 50013,
|
||||||
|
status: 403,
|
||||||
|
});
|
||||||
|
postMock.mockRejectedValueOnce(apiError);
|
||||||
|
getMock
|
||||||
|
.mockResolvedValueOnce({ type: ChannelType.GuildText })
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
id: "789",
|
||||||
|
guild_id: "guild1",
|
||||||
|
type: 0,
|
||||||
|
permission_overwrites: [],
|
||||||
|
})
|
||||||
|
.mockResolvedValueOnce({ id: "bot1" })
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
id: "guild1",
|
||||||
|
roles: [{ id: "guild1", permissions: perms.toString() }],
|
||||||
|
})
|
||||||
|
.mockResolvedValueOnce({ roles: [] });
|
||||||
|
|
||||||
|
let error: unknown;
|
||||||
|
try {
|
||||||
|
await sendMessageDiscord("channel:789", "hello", { rest, token: "t", cfg: DISCORD_TEST_CFG });
|
||||||
|
} catch (err) {
|
||||||
|
error = err;
|
||||||
|
}
|
||||||
|
expect(String(error)).toMatch(
|
||||||
|
/permission probe did not identify missing ViewChannel\/SendMessages/,
|
||||||
|
);
|
||||||
|
expect(String(error)).toMatch(/code=50013 status=403/);
|
||||||
|
});
|
||||||
|
|
||||||
it("uploads media attachments", async () => {
|
it("uploads media attachments", async () => {
|
||||||
const { rest, postMock } = makeDiscordRest();
|
const { rest, postMock } = makeDiscordRest();
|
||||||
postMock.mockResolvedValue({ id: "msg", channel_id: "789" });
|
postMock.mockResolvedValue({ id: "msg", channel_id: "789" });
|
||||||
|
|||||||
@@ -116,6 +116,25 @@ function getDiscordErrorCode(err: unknown) {
|
|||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getDiscordErrorStatus(err: unknown) {
|
||||||
|
if (!err || typeof err !== "object") {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
const candidate =
|
||||||
|
"status" in err && err.status !== undefined
|
||||||
|
? err.status
|
||||||
|
: "statusCode" in err && err.statusCode !== undefined
|
||||||
|
? err.statusCode
|
||||||
|
: undefined;
|
||||||
|
if (typeof candidate === "number" && Number.isFinite(candidate)) {
|
||||||
|
return candidate;
|
||||||
|
}
|
||||||
|
if (typeof candidate === "string" && /^\d+$/.test(candidate)) {
|
||||||
|
return Number(candidate);
|
||||||
|
}
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
async function buildDiscordSendError(
|
async function buildDiscordSendError(
|
||||||
err: unknown,
|
err: unknown,
|
||||||
ctx: {
|
ctx: {
|
||||||
@@ -132,8 +151,8 @@ async function buildDiscordSendError(
|
|||||||
const code = getDiscordErrorCode(err);
|
const code = getDiscordErrorCode(err);
|
||||||
if (code === DISCORD_CANNOT_DM) {
|
if (code === DISCORD_CANNOT_DM) {
|
||||||
return new DiscordSendError(
|
return new DiscordSendError(
|
||||||
"discord dm failed: user blocks dms or privacy settings disallow it",
|
`discord dm failed: user blocks dms or privacy settings disallow it (code=${code})`,
|
||||||
{ kind: "dm-blocked" },
|
{ kind: "dm-blocked", discordCode: code, status: getDiscordErrorStatus(err) },
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (code !== DISCORD_MISSING_PERMISSIONS) {
|
if (code !== DISCORD_MISSING_PERMISSIONS) {
|
||||||
@@ -141,15 +160,17 @@ async function buildDiscordSendError(
|
|||||||
}
|
}
|
||||||
|
|
||||||
let missing: string[] = [];
|
let missing: string[] = [];
|
||||||
|
let probedChannelType: number | undefined;
|
||||||
try {
|
try {
|
||||||
const permissions = await fetchChannelPermissionsDiscord(ctx.channelId, {
|
const permissions = await fetchChannelPermissionsDiscord(ctx.channelId, {
|
||||||
rest: ctx.rest,
|
rest: ctx.rest,
|
||||||
token: ctx.token,
|
token: ctx.token,
|
||||||
cfg: ctx.cfg,
|
cfg: ctx.cfg,
|
||||||
});
|
});
|
||||||
|
probedChannelType = permissions.channelType;
|
||||||
const current = new Set(permissions.permissions);
|
const current = new Set(permissions.permissions);
|
||||||
const required = ["ViewChannel", "SendMessages"];
|
const required = ["ViewChannel", "SendMessages"];
|
||||||
if (isThreadChannelType(permissions.channelType)) {
|
if (isThreadChannelType(probedChannelType)) {
|
||||||
required.push("SendMessagesInThreads");
|
required.push("SendMessagesInThreads");
|
||||||
}
|
}
|
||||||
if (ctx.hasMedia) {
|
if (ctx.hasMedia) {
|
||||||
@@ -160,15 +181,29 @@ async function buildDiscordSendError(
|
|||||||
/* ignore permission probe errors */
|
/* ignore permission probe errors */
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const status = getDiscordErrorStatus(err);
|
||||||
|
const apiDetails = [`code=${code}`, status != null ? `status=${status}` : undefined]
|
||||||
|
.filter(Boolean)
|
||||||
|
.join(" ");
|
||||||
|
const probedPermissions = ["ViewChannel", "SendMessages"];
|
||||||
|
if (isThreadChannelType(probedChannelType)) {
|
||||||
|
probedPermissions.push("SendMessagesInThreads");
|
||||||
|
}
|
||||||
|
if (ctx.hasMedia) {
|
||||||
|
probedPermissions.push("AttachFiles");
|
||||||
|
}
|
||||||
|
const probeSummary = probedPermissions.join("/");
|
||||||
const missingLabel = missing.length
|
const missingLabel = missing.length
|
||||||
? `missing permissions in channel ${ctx.channelId}: ${missing.join(", ")}`
|
? `discord missing permissions in channel ${ctx.channelId}: ${missing.join(", ")}`
|
||||||
: `missing permissions in channel ${ctx.channelId}`;
|
: `discord missing permissions in channel ${ctx.channelId}; permission probe did not identify missing ${probeSummary}`;
|
||||||
return new DiscordSendError(
|
return new DiscordSendError(
|
||||||
`${missingLabel}. bot might be muted or blocked by role/channel overrides`,
|
`${missingLabel} (${apiDetails}). bot might be blocked by channel/thread overrides, archived thread state, reply target visibility, or app-role position`,
|
||||||
{
|
{
|
||||||
kind: "missing-permissions",
|
kind: "missing-permissions",
|
||||||
channelId: ctx.channelId,
|
channelId: ctx.channelId,
|
||||||
missingPermissions: missing,
|
missingPermissions: missing,
|
||||||
|
discordCode: code,
|
||||||
|
status,
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ export class DiscordSendError extends Error {
|
|||||||
kind?: "missing-permissions" | "dm-blocked";
|
kind?: "missing-permissions" | "dm-blocked";
|
||||||
channelId?: string;
|
channelId?: string;
|
||||||
missingPermissions?: string[];
|
missingPermissions?: string[];
|
||||||
|
discordCode?: number;
|
||||||
|
status?: number;
|
||||||
|
|
||||||
constructor(message: string, opts?: Partial<DiscordSendError>) {
|
constructor(message: string, opts?: Partial<DiscordSendError>) {
|
||||||
super(message);
|
super(message);
|
||||||
|
|||||||
Reference in New Issue
Block a user