diff --git a/src/app/features/room/RoomInput.tsx b/src/app/features/room/RoomInput.tsx index c3562a955..667aaf12f 100644 --- a/src/app/features/room/RoomInput.tsx +++ b/src/app/features/room/RoomInput.tsx @@ -113,7 +113,14 @@ import { } from './msgContent'; import { getMemberName, getMentionContent, trimReplyFromBody } from '../../utils/room'; 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 { useElementSizeObserver } from '../../hooks/useElementSizeObserver'; import { ReplyLayout, ThreadIndicator } from '../../components/message'; @@ -665,16 +672,16 @@ export const RoomInput = forwardRef( const commandContent = commands[commandName as Command]; if (commandContent) { // Fire-and-forget by design (the editor resets immediately for UX), but - // surface a rejection instead of failing silently. NOTE: /kick and /ban - // route through rateLimitedActions (utils/matrix.ts), whose to() helper - // swallows non-429 errors, so those two commands can still resolve even - // when the underlying kick/ban failed — this catch only covers errors - // that actually reject out of exe(). + // surface a rejection instead of failing silently. /kick and /ban + // throw a CommandError whose message already names who and why + // (#216); anything else gets the generic sentence. commandContent.exe(plainText).catch((err) => { console.error(`Failed to run /${commandName} command:`, err); setToast( createErrorToast( - `The /${commandName} command failed. Please try again.`, + err instanceof CommandError + ? err.message + : `The /${commandName} command failed. Please try again.`, Icons.Warning, 'Command failed', ), diff --git a/src/app/hooks/useCommands.ts b/src/app/hooks/useCommands.ts index 2fbf996fd..c467542ac 100644 --- a/src/app/hooks/useCommands.ts +++ b/src/app/hooks/useCommands.ts @@ -20,6 +20,7 @@ import { isServerName, isUserId, rateLimitedActions, + RateLimitedFailure, removeRoomIdFromMDirect, } from '../utils/matrix'; import { useRoomNavigate } from './useRoomNavigate'; @@ -138,6 +139,28 @@ export const parseTimestampFlag = (input: string): number | undefined => { export type CommandExe = (payload: string) => Promise; +/** + * [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[], +): 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 { Me = 'me', Notice = 'notice', @@ -292,11 +315,13 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => { }); } - // NOTE: rateLimitedActions' to() helper (utils/matrix.ts) swallows every - // non-429 error, so a failed kick (e.g. insufficient power level) resolves - // silently here — the RoomInput exe() catch cannot surface it. Propagating - // would require refactoring rateLimitedActions' shared error handling. - await rateLimitedActions(users, (id) => mx.kick(room.roomId, id, reason)); + // [Gitea #216] rateLimitedActions collects per-user failures (it + // used to swallow them); turn them into one CommandError so the + // RoomInput toast can say who and why. + const { failures } = await rateLimitedActions(users, (id) => + mx.kick(room.roomId, id, reason), + ); + throwCommandFailures('kick', failures); }, }, [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 - // failed ban resolves silently and can't be surfaced by the exe() catch. - await rateLimitedActions(users, (id) => mx.ban(room.roomId, id, reason)); + const { failures } = await rateLimitedActions(users, (id) => + mx.ban(room.roomId, id, reason), + ); + throwCommandFailures('ban', failures); }, }, [Command.UnBan]: { diff --git a/src/app/utils/matrix.ts b/src/app/utils/matrix.ts index 1471325fd..33e35e501 100644 --- a/src/app/utils/matrix.ts +++ b/src/app/utils/matrix.ts @@ -467,15 +467,24 @@ export const declineInvite = async (mx: MatrixClient, roomId: string): Promise undefined); }; +export type RateLimitedFailure = { 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 ( data: T[], callback: (item: T, index: number) => Promise, maxRetryCount?: number, -) => { +): Promise<{ failures: RateLimitedFailure[] }> => { let retryCount = 0; let actionInterval = 0; + const failures: RateLimitedFailure[] = []; + const sleepForMs = (ms: number) => new Promise((resolve) => { setTimeout(resolve, ms); @@ -486,6 +495,7 @@ export const rateLimitedActions = async ( if (err?.httpStatus === 429) { if (retryCount === maxRetryCount) { + failures.push({ item: dataItem, error: err }); return; } @@ -496,6 +506,8 @@ export const rateLimitedActions = async ( retryCount += 1; await performAction(dataItem, index); + } else if (err) { + failures.push({ item: dataItem, error: err }); } }; @@ -509,6 +521,7 @@ export const rateLimitedActions = async ( await sleepForMs(actionInterval); } } + return { failures }; }; export const knockSupported = (version: string): boolean => {