fix(commands): /kick and /ban failures are no longer silent (#216)
rateLimitedActions now collects non-429 failures (and a 429 that exhausted its retries) and returns them instead of swallowing them; existing callers ignore the return. /kick and /ban turn the list into a CommandError whose message names who and why, using the server's own sentence (MatrixError.data.error), never the URL-bearing toString(); RoomInput's toast shows it verbatim. Verified headless as a non-moderator: '/kick @alice' → "Could not kick @alice:localhost: You cannot kick user @alice:localhost."; '/ban @nobody @alice' → "Could not ban @nobody:localhost, @alice:localhost: You don't have permission to ban". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -113,7 +113,14 @@ import {
|
|||||||
} from './msgContent';
|
} from './msgContent';
|
||||||
import { getMemberName, getMentionContent, trimReplyFromBody } from '../../utils/room';
|
import { getMemberName, getMentionContent, trimReplyFromBody } from '../../utils/room';
|
||||||
import { CommandAutocomplete } from './CommandAutocomplete';
|
import { CommandAutocomplete } from './CommandAutocomplete';
|
||||||
import { Command, SHRUG, TABLEFLIP, UNFLIP, useCommands } from '../../hooks/useCommands';
|
import {
|
||||||
|
Command,
|
||||||
|
CommandError,
|
||||||
|
SHRUG,
|
||||||
|
TABLEFLIP,
|
||||||
|
UNFLIP,
|
||||||
|
useCommands,
|
||||||
|
} from '../../hooks/useCommands';
|
||||||
import { mobileOrTablet } from '../../utils/user-agent';
|
import { mobileOrTablet } from '../../utils/user-agent';
|
||||||
import { useElementSizeObserver } from '../../hooks/useElementSizeObserver';
|
import { useElementSizeObserver } from '../../hooks/useElementSizeObserver';
|
||||||
import { ReplyLayout, ThreadIndicator } from '../../components/message';
|
import { ReplyLayout, ThreadIndicator } from '../../components/message';
|
||||||
@@ -665,16 +672,16 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
|
|||||||
const commandContent = commands[commandName as Command];
|
const commandContent = commands[commandName as Command];
|
||||||
if (commandContent) {
|
if (commandContent) {
|
||||||
// Fire-and-forget by design (the editor resets immediately for UX), but
|
// Fire-and-forget by design (the editor resets immediately for UX), but
|
||||||
// surface a rejection instead of failing silently. NOTE: /kick and /ban
|
// surface a rejection instead of failing silently. /kick and /ban
|
||||||
// route through rateLimitedActions (utils/matrix.ts), whose to() helper
|
// throw a CommandError whose message already names who and why
|
||||||
// swallows non-429 errors, so those two commands can still resolve even
|
// (#216); anything else gets the generic sentence.
|
||||||
// when the underlying kick/ban failed — this catch only covers errors
|
|
||||||
// that actually reject out of exe().
|
|
||||||
commandContent.exe(plainText).catch((err) => {
|
commandContent.exe(plainText).catch((err) => {
|
||||||
console.error(`Failed to run /${commandName} command:`, err);
|
console.error(`Failed to run /${commandName} command:`, err);
|
||||||
setToast(
|
setToast(
|
||||||
createErrorToast(
|
createErrorToast(
|
||||||
`The /${commandName} command failed. Please try again.`,
|
err instanceof CommandError
|
||||||
|
? err.message
|
||||||
|
: `The /${commandName} command failed. Please try again.`,
|
||||||
Icons.Warning,
|
Icons.Warning,
|
||||||
'Command failed',
|
'Command failed',
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ import {
|
|||||||
isServerName,
|
isServerName,
|
||||||
isUserId,
|
isUserId,
|
||||||
rateLimitedActions,
|
rateLimitedActions,
|
||||||
|
RateLimitedFailure,
|
||||||
removeRoomIdFromMDirect,
|
removeRoomIdFromMDirect,
|
||||||
} from '../utils/matrix';
|
} from '../utils/matrix';
|
||||||
import { useRoomNavigate } from './useRoomNavigate';
|
import { useRoomNavigate } from './useRoomNavigate';
|
||||||
@@ -138,6 +139,28 @@ export const parseTimestampFlag = (input: string): number | undefined => {
|
|||||||
|
|
||||||
export type CommandExe = (payload: string) => Promise<void>;
|
export type CommandExe = (payload: string) => Promise<void>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* [Gitea #216] A command that partially or fully failed. `message` is written
|
||||||
|
* for the user (the RoomInput toast shows it verbatim).
|
||||||
|
*/
|
||||||
|
export class CommandError extends Error {}
|
||||||
|
|
||||||
|
const throwCommandFailures = (
|
||||||
|
verb: 'kick' | 'ban',
|
||||||
|
failures: RateLimitedFailure<string>[],
|
||||||
|
): void => {
|
||||||
|
if (failures.length === 0) return;
|
||||||
|
const who = failures.map((f) => f.item).join(', ');
|
||||||
|
// MatrixError.data.error is the server's sentence ("You don't have
|
||||||
|
// permission to kick"); fall back to the errcode, never the URL-bearing
|
||||||
|
// toString().
|
||||||
|
const reason =
|
||||||
|
(failures[0].error.data as { error?: string } | undefined)?.error ??
|
||||||
|
failures[0].error.errcode ??
|
||||||
|
'unknown error';
|
||||||
|
throw new CommandError(`Could not ${verb} ${who}: ${reason}`);
|
||||||
|
};
|
||||||
|
|
||||||
export enum Command {
|
export enum Command {
|
||||||
Me = 'me',
|
Me = 'me',
|
||||||
Notice = 'notice',
|
Notice = 'notice',
|
||||||
@@ -292,11 +315,13 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// NOTE: rateLimitedActions' to() helper (utils/matrix.ts) swallows every
|
// [Gitea #216] rateLimitedActions collects per-user failures (it
|
||||||
// non-429 error, so a failed kick (e.g. insufficient power level) resolves
|
// used to swallow them); turn them into one CommandError so the
|
||||||
// silently here — the RoomInput exe() catch cannot surface it. Propagating
|
// RoomInput toast can say who and why.
|
||||||
// would require refactoring rateLimitedActions' shared error handling.
|
const { failures } = await rateLimitedActions(users, (id) =>
|
||||||
await rateLimitedActions(users, (id) => mx.kick(room.roomId, id, reason));
|
mx.kick(room.roomId, id, reason),
|
||||||
|
);
|
||||||
|
throwCommandFailures('kick', failures);
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
[Command.Ban]: {
|
[Command.Ban]: {
|
||||||
@@ -318,9 +343,10 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// See the /kick note: rateLimitedActions swallows non-429 errors, so a
|
const { failures } = await rateLimitedActions(users, (id) =>
|
||||||
// failed ban resolves silently and can't be surfaced by the exe() catch.
|
mx.ban(room.roomId, id, reason),
|
||||||
await rateLimitedActions(users, (id) => mx.ban(room.roomId, id, reason));
|
);
|
||||||
|
throwCommandFailures('ban', failures);
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
[Command.UnBan]: {
|
[Command.UnBan]: {
|
||||||
|
|||||||
+14
-1
@@ -467,15 +467,24 @@ export const declineInvite = async (mx: MatrixClient, roomId: string): Promise<v
|
|||||||
await mx.forget(roomId).catch(() => undefined);
|
await mx.forget(roomId).catch(() => undefined);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
export type RateLimitedFailure<T> = { item: T; error: MatrixError };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Run `callback` over `data` sequentially, backing off on 429. Other errors do
|
||||||
|
* not stop the loop — they are collected and returned so callers can tell the
|
||||||
|
* user which items failed (a failed /kick used to vanish silently, #216).
|
||||||
|
*/
|
||||||
export const rateLimitedActions = async <T, R = void>(
|
export const rateLimitedActions = async <T, R = void>(
|
||||||
data: T[],
|
data: T[],
|
||||||
callback: (item: T, index: number) => Promise<R>,
|
callback: (item: T, index: number) => Promise<R>,
|
||||||
maxRetryCount?: number,
|
maxRetryCount?: number,
|
||||||
) => {
|
): Promise<{ failures: RateLimitedFailure<T>[] }> => {
|
||||||
let retryCount = 0;
|
let retryCount = 0;
|
||||||
|
|
||||||
let actionInterval = 0;
|
let actionInterval = 0;
|
||||||
|
|
||||||
|
const failures: RateLimitedFailure<T>[] = [];
|
||||||
|
|
||||||
const sleepForMs = (ms: number) =>
|
const sleepForMs = (ms: number) =>
|
||||||
new Promise((resolve) => {
|
new Promise((resolve) => {
|
||||||
setTimeout(resolve, ms);
|
setTimeout(resolve, ms);
|
||||||
@@ -486,6 +495,7 @@ export const rateLimitedActions = async <T, R = void>(
|
|||||||
|
|
||||||
if (err?.httpStatus === 429) {
|
if (err?.httpStatus === 429) {
|
||||||
if (retryCount === maxRetryCount) {
|
if (retryCount === maxRetryCount) {
|
||||||
|
failures.push({ item: dataItem, error: err });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -496,6 +506,8 @@ export const rateLimitedActions = async <T, R = void>(
|
|||||||
retryCount += 1;
|
retryCount += 1;
|
||||||
|
|
||||||
await performAction(dataItem, index);
|
await performAction(dataItem, index);
|
||||||
|
} else if (err) {
|
||||||
|
failures.push({ item: dataItem, error: err });
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -509,6 +521,7 @@ export const rateLimitedActions = async <T, R = void>(
|
|||||||
await sleepForMs(actionInterval);
|
await sleepForMs(actionInterval);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
return { failures };
|
||||||
};
|
};
|
||||||
|
|
||||||
export const knockSupported = (version: string): boolean => {
|
export const knockSupported = (version: string): boolean => {
|
||||||
|
|||||||
Reference in New Issue
Block a user