diff --git a/.changeset/failed-send-state.md b/.changeset/failed-send-state.md new file mode 100644 index 0000000..43ff12e --- /dev/null +++ b/.changeset/failed-send-state.md @@ -0,0 +1,5 @@ +--- +"@zooid/web": patch +--- + +A message the server rejects (for example sending from a room you've only been invited to) now shows as "Not sent" on its own tile with the reason, a Retry button that reuses the same transaction ID, and a Delete button, instead of looking delivered with a raw error under the composer. diff --git a/packages/web/src/components/rooms/composer.failed-send.integration.test.tsx b/packages/web/src/components/rooms/composer.failed-send.integration.test.tsx new file mode 100644 index 0000000..58dc352 --- /dev/null +++ b/packages/web/src/components/rooms/composer.failed-send.integration.test.tsx @@ -0,0 +1,140 @@ +import { cleanup, render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { EventStatus, MatrixError, MatrixEvent, type Room } from "matrix-js-sdk"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { MatrixClientPeg } from "@/client/peg"; +import { useTimeline } from "@/hooks/use-timeline"; +import { makeFakeClient, makeRoom } from "../../../test/factories"; +import { MessagePanel } from "../structures/message-panel"; +import { Composer } from "./composer"; + +const roomId = "!r:h.example"; +const me = "@me:h.example"; +const TXN = "txn-1"; + +afterEach(() => { + cleanup(); + MatrixClientPeg.reset(); +}); + +function Harness({ thread }: { thread?: string }) { + const { events } = useTimeline(roomId); + return ( + <> + + + + ); +} + +/** Mimics what client.sendEvent does: add a local echo, then reject and mark it NOT_SENT. */ +function rejectingSendEvent(room: Room, error: Error) { + return vi.fn(async (_roomId: string, _thread: string | null, type: string, content: object) => { + const echo = new MatrixEvent({ + type, + content, + sender: me, + room_id: roomId, + event_id: `~${roomId}:${TXN}`, + origin_server_ts: Date.now(), + }); + echo.setTxnId(TXN); + echo.setStatus(EventStatus.SENDING); + room.addPendingEvent(echo, TXN); + echo.error = error as MatrixError; + room.updatePendingEvent(echo, EventStatus.NOT_SENT); + throw error; + }); +} + +function setup(error: Error) { + const client = makeFakeClient({ userId: me }); + const room = makeRoom(roomId, { client, myUserId: me }); + const cast = client as unknown as Record; + cast.getRoom = () => room; + cast.sendEvent = rejectingSendEvent(room, error); + cast.resendEvent = vi.fn().mockResolvedValue({ event_id: "$sent" }); + cast.cancelPendingEvent = vi.fn(); + MatrixClientPeg.injectClientForTest(client); + return { cast, room }; +} + +const forbidden = () => + new MatrixError( + { errcode: "M_FORBIDDEN", error: "Auth check failed: sender's membership 'invite' is not 'join'" }, + 403, + ); + +async function sendHello() { + const user = userEvent.setup(); + await user.type(screen.getByRole("textbox", { name: /message/i }), "@agent hello{Enter}"); + return user; +} + +describe("a send the server rejects", () => { + it("shows the failed tile with Retry and no raw MatrixError under the composer", async () => { + setup(forbidden()); + render(); + await sendHello(); + + const alert = await screen.findByRole("alert"); + expect(within(alert).getByText("Not sent")).toBeInTheDocument(); + expect(within(alert).getByText(/not a member of this room yet/i)).toBeInTheDocument(); + expect(within(alert).getByRole("button", { name: "Retry" })).toBeInTheDocument(); + expect(within(alert).getByRole("button", { name: "Delete" })).toBeInTheDocument(); + // the message text stays on the tile, and nothing renders it as delivered + expect(screen.getByText(/hello/)).toBeInTheDocument(); + expect(document.querySelector("time")).toBeNull(); + // the composer cleared and carries no raw error line + expect(screen.getByRole("textbox", { name: /message/i })).toHaveValue(""); + expect(screen.queryByText(/MatrixError|M_FORBIDDEN|Auth check failed/)).toBeNull(); + }); + + it("Retry resends the same event, so the txnId is reused", async () => { + const { cast, room } = setup(forbidden()); + render(); + const user = await sendHello(); + + await user.click(await screen.findByRole("button", { name: "Retry" })); + + const echo = room.getLiveTimeline().getEvents()[0]; + expect(echo.getAssociatedStatus() ?? echo.status).toBe(EventStatus.NOT_SENT); + expect(cast.resendEvent).toHaveBeenCalledTimes(1); + const [event, inRoom] = (cast.resendEvent as ReturnType).mock.calls[0]; + expect(event).toBe(echo); + expect(event.getTxnId()).toBe(TXN); + expect(inRoom).toBe(room); + expect(cast.sendEvent).toHaveBeenCalledTimes(1); + }); + + it("Delete cancels the pending event", async () => { + const { cast, room } = setup(forbidden()); + render(); + const user = await sendHello(); + + await user.click(await screen.findByRole("button", { name: "Delete" })); + expect(cast.cancelPendingEvent).toHaveBeenCalledWith(room.getLiveTimeline().getEvents()[0]); + }); + + it("repaints the tile when the echo goes back to sending", async () => { + const { room } = setup(forbidden()); + render(); + await sendHello(); + await screen.findByRole("alert"); + + room.updatePendingEvent(room.getLiveTimeline().getEvents()[0], EventStatus.SENDING); + await waitFor(() => expect(screen.queryByRole("alert")).toBeNull()); + expect(screen.getByText("Sending…")).toBeInTheDocument(); + }); + + it("keeps the composer error for a custom event, which has no tile to show it", async () => { + const { cast } = setup(forbidden()); + render(); + const user = userEvent.setup(); + await user.type(screen.getByRole("textbox", { name: /message/i }), "/clear{Enter}"); + + expect(await screen.findByText(/MatrixError: \[403\]/)).toBeInTheDocument(); + expect(screen.queryByText("Not sent")).toBeNull(); + expect(cast.cancelPendingEvent).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/web/src/components/rooms/composer.tsx b/packages/web/src/components/rooms/composer.tsx index 84860a8..a812894 100644 --- a/packages/web/src/components/rooms/composer.tsx +++ b/packages/web/src/components/rooms/composer.tsx @@ -1,11 +1,12 @@ import { useMemo, useState } from "react"; +import { EventStatus, type MatrixEvent, type Room } from "matrix-js-sdk"; import { parseSlashCommand } from "@/lib/slash-commands"; import { buildQuoteContent } from "@/lib/matrix/quote"; import { setQuoteDraft, useQuoteDraft } from "@/lib/quote-draft-store"; import { QuoteChip } from "./quote-chip"; import { MessageInput, type MessageInputSubmit } from "./message-input"; import { useMatrixClient } from "../../hooks/use-matrix-client"; -import { useThreadPreview } from "../../hooks/use-timeline"; +import { allRoomEvents, useThreadPreview } from "../../hooks/use-timeline"; import { useTyping } from "../../hooks/use-typing"; import { useMediaUpload } from "../../hooks/use-media-upload"; @@ -27,6 +28,9 @@ type SendEvent = ( content: Record, ) => Promise<{ event_id: string }>; +const notSentEchoes = (room: Room | null): MatrixEvent[] => + room ? allRoomEvents(room).filter((ev) => ev.status === EventStatus.NOT_SENT) : []; + export function Composer({ roomId, threadRootEventId, onExitThread }: ComposerProps) { const client = useMatrixClient(); const [error, setError] = useState(null); @@ -53,6 +57,30 @@ export function Composer({ roomId, threadRootEventId, onExitThread }: ComposerPr return (rootEvt?.getContent() as { body?: string } | undefined)?.body ?? ""; }, [lastThreadEvent, threadRootEventId, client, roomId]); + /** + * Send one event. When the server rejects it, the SDK keeps a NOT_SENT local + * echo in the timeline. For message content that tile owns the failure (reason, + * Retry, Delete), so the error is swallowed here. Custom event types have no + * tile that shows a failure, so their echo is dropped and the error rethrown + * for the composer's error line. + */ + async function sendEvent( + type: string, + content: Record, + { tile }: { tile: boolean }, + ): Promise { + const room = client.getRoom(roomId); + const known = new Set(notSentEchoes(room)); + try { + await (client.sendEvent as unknown as SendEvent).call(client, roomId, threadId, type, content); + } catch (err) { + const echoes = notSentEchoes(room).filter((ev) => !known.has(ev)); + if (tile && echoes.length > 0) return; + for (const ev of echoes) client.cancelPendingEvent(ev); + throw err; + } + } + async function send({ body, rawBody, mentionUserIds, attachments, setAttachments }: MessageInputSubmit): Promise { // Slash commands only apply when there's no attachment and the body starts with / if (rawBody && attachments.length === 0 && !quoteDraft) { @@ -67,13 +95,7 @@ export function Composer({ roomId, threadRootEventId, onExitThread }: ComposerPr "m.relates_to": { rel_type: "m.thread", event_id: threadId }, } : slash.content; - await (client.sendEvent as unknown as SendEvent).call( - client, - roomId, - threadId, - slash.eventType, - slashContent, - ); + await sendEvent(slash.eventType, slashContent, { tile: false }); return; } } @@ -96,13 +118,7 @@ export function Composer({ roomId, threadRootEventId, onExitThread }: ComposerPr info: { mimetype: file.type, size: file.size }, }; if (!isImage) mediaContent.filename = file.name; - await (client.sendEvent as unknown as SendEvent).call( - client, - roomId, - threadId, - "m.room.message", - mediaContent, - ); + await sendEvent("m.room.message", mediaContent, { tile: true }); pending.shift(); } } finally { @@ -123,13 +139,7 @@ export function Composer({ roomId, threadRootEventId, onExitThread }: ComposerPr if (mentionUserIds.length > 0) { content["m.mentions"] = { user_ids: mentionUserIds }; } - await (client.sendEvent as unknown as SendEvent).call( - client, - roomId, - threadId, - "m.room.message", - content, - ); + await sendEvent("m.room.message", content, { tile: true }); if (quoteDraft) setQuoteDraft(roomId, threadId, null); } } @@ -140,12 +150,10 @@ export function Composer({ roomId, threadRootEventId, onExitThread }: ComposerPr if (!slash) return; setError(null); try { - await (client.sendEvent as unknown as SendEvent).call( - client, - roomId, - threadId, + await sendEvent( slash.eventType, { ...slash.content, "m.relates_to": { rel_type: "m.thread", event_id: threadId } }, + { tile: false }, ); } catch (err) { setError(err instanceof Error ? err.message : String(err)); diff --git a/packages/web/src/components/timeline/event-tile.tsx b/packages/web/src/components/timeline/event-tile.tsx index 6487755..2cb36df 100644 --- a/packages/web/src/components/timeline/event-tile.tsx +++ b/packages/web/src/components/timeline/event-tile.tsx @@ -1,6 +1,6 @@ import { ElicitationEventType } from "../../events/elicitation"; import { QuestionCard } from "./question-card"; -import type { MatrixEvent } from "matrix-js-sdk"; +import { EventStatus, type MatrixEvent } from "matrix-js-sdk"; import { ApprovalEventType } from "../../events/approval"; import { decodeZooidEvent, isZooidLifecycle } from "../../events/zooid-events"; import { ApprovalCard } from "./approval-card"; @@ -8,6 +8,7 @@ import { ZooidEventTile } from "./zooid-event"; import { ErrorTile } from "./error-tile"; import { MediaMessage } from "./media-message"; import { MembershipEvent } from "./membership-event"; +import { EventSendFailure } from "./send-state"; import { SessionDivider } from "./session-divider"; import { TextMessage } from "./text-message"; @@ -29,7 +30,12 @@ export function EventTile({ if (event.getType() === "m.room.message") { const msgtype = (event.getContent() as { msgtype?: string }).msgtype; if (msgtype && MEDIA_MSGTYPES.has(msgtype)) { - return ; + return ( +
+ + {event.status === EventStatus.NOT_SENT && } +
+ ); } return ( } + senderName="me" + senderColor={senderColor(me)} + senderTitle={me} + timestamp={ + state === "failed" ? undefined : state === "sending" ? ( + + ) : ( + + ) + } + > +

{body}

+ {state === "failed" && } + + ); +} + +const meta = { + title: "Timeline/Send state", + component: SendFailure, + parameters: { layout: "padded" }, +} satisfies Meta; + +export default meta; + +const notJoined = describeSendError({ + errcode: "M_FORBIDDEN", + data: { error: "Auth check failed: sender's membership 'invite' is not 'join'" }, +}); + +// Custom render ignores the SendFailure args; each story builds its tile inline. +export const Sending = { + args: { reason: "", onRetry: fn(), onDelete: fn() }, + render: () => ( +
+ +
+ ), +}; + +export const FailedNotJoined = { + args: { reason: notJoined, onRetry: fn(), onDelete: fn() }, + render: () => ( +
+ +
+ ), +}; + +export const FailedOtherError = { + args: { reason: "", onRetry: fn(), onDelete: fn() }, + render: () => ( +
+ +
+ ), +}; + +// The three states side by side with a delivered message for contrast. +export const InTimeline = { + args: { reason: "", onRetry: fn(), onDelete: fn() }, + render: () => ( +
    + + + +
+ ), +}; diff --git a/packages/web/src/components/timeline/send-state.tsx b/packages/web/src/components/timeline/send-state.tsx new file mode 100644 index 0000000..f8cc3ee --- /dev/null +++ b/packages/web/src/components/timeline/send-state.tsx @@ -0,0 +1,61 @@ +import type { MatrixEvent } from "matrix-js-sdk"; +import { TriangleAlertIcon } from "lucide-react"; +import { useMatrixClient } from "@/hooks/use-matrix-client"; + +/** Short, human reason a send was rejected, from the error the SDK kept on the echo. */ +export function describeSendError(err: unknown): string { + const e = err as { errcode?: string; message?: string; data?: { error?: string } } | null | undefined; + const detail = e?.data?.error ?? e?.message ?? ""; + if (e?.errcode === "M_FORBIDDEN") { + return /membership/i.test(detail) + ? "You're not a member of this room yet" + : "You don't have permission to send here"; + } + if (e?.errcode) return e.errcode.replace(/^M_/, "").toLowerCase().replace(/_/g, " "); + return detail || "Couldn't reach the server"; +} + +export function SendingIndicator() { + return Sending…; +} + +export function SendFailure({ + reason, + onRetry, + onDelete, +}: { + reason: string; + onRetry: () => void; + onDelete: () => void; +}) { + return ( +
+ + Not sent + · {reason} + + + + +
+ ); +} + +/** Failure line for a local echo the server rejected; Retry reuses its txnId. */ +export function EventSendFailure({ event }: { event: MatrixEvent }) { + const client = useMatrixClient(); + const room = client?.getRoom(event.getRoomId() ?? ""); + return ( + { + if (client && room) void client.resendEvent(event, room).catch(() => {}); + }} + onDelete={() => client?.cancelPendingEvent(event)} + /> + ); +} diff --git a/packages/web/src/components/timeline/text-message.tsx b/packages/web/src/components/timeline/text-message.tsx index 594da9d..a6ef5d3 100644 --- a/packages/web/src/components/timeline/text-message.tsx +++ b/packages/web/src/components/timeline/text-message.tsx @@ -1,6 +1,6 @@ import { useState, useRef, useEffect } from "react"; import { useIsMobile } from "@/hooks/use-mobile"; -import type { MatrixEvent } from "matrix-js-sdk"; +import { EventStatus, type MatrixEvent } from "matrix-js-sdk"; import { MessageSquare, TriangleAlertIcon } from "lucide-react"; import { toast } from "sonner"; import { senderColor, splitMentions } from "@/lib/sender"; @@ -17,6 +17,7 @@ import { ZooidEventType } from "@/events/zooid-events"; import { FormattedMessageBody } from "./formatted-message-body"; import { MessageTile } from "./message-tile"; import { MessageTimestamp } from "./message-timestamp"; +import { EventSendFailure, SendingIndicator } from "./send-state"; import { ReactionPicker } from "./reaction-picker"; import { ReactionsRow } from "./reactions-row"; import { ReadReceiptsRow } from "./read-receipts-row"; @@ -169,6 +170,11 @@ export function TextMessage({ ? c.format === "org.matrix.custom.html" && typeof displayFormattedBody === "string" && displayFormattedBody.length > 0 : c.format === "org.matrix.custom.html" && typeof c.formatted_body === "string" && c.formatted_body.length > 0); const quote = readQuoteRef(c); + const failed = event.status === EventStatus.NOT_SENT; + const sending = + event.status === EventStatus.SENDING || + event.status === EventStatus.QUEUED || + event.status === EventStatus.ENCRYPTING; const { comment, fallback } = quote ? splitQuoteFallback(displayBody) : { comment: displayBody, fallback: "" }; @@ -291,13 +297,13 @@ export function TextMessage({ ref={wrapperRef} data-selected={selected || undefined} onClick={() => { if (isMobile) setSelected(true); }} - className="hover:bg-muted/30 data-[selected]:bg-muted/30" + className={`hover:bg-muted/30 data-[selected]:bg-muted/30 ${failed ? "opacity-60" : ""}`} avatar={} senderName={senderName} senderColor={senderColor(sender)} senderTitle={sender} - timestamp={} - actions={actions} + timestamp={failed ? undefined : sending ? : } + actions={failed || sending ? undefined : actions} > {editing ? ( + )} + diff --git a/packages/web/src/hooks/use-timeline.ts b/packages/web/src/hooks/use-timeline.ts index fbe8c39..d78026d 100644 --- a/packages/web/src/hooks/use-timeline.ts +++ b/packages/web/src/hooks/use-timeline.ts @@ -23,6 +23,12 @@ interface TimelineState { * not adjacent — messages are missing in between and have to be paginated in. */ gapBeforeEventIds: string[]; + /** + * `id:status` of every local echo. A status change (SENDING → NOT_SENT → + * SENDING → sent) mutates the same MatrixEvent in place, so without this the + * cached snapshot would look unchanged and the tile would never repaint. + */ + echoKey: string; } export interface ThreadPreviewState { @@ -43,7 +49,7 @@ export interface ThreadFullState { totalCount: number; } -const EMPTY: TimelineState = { events: [], pendingRootIds: [], gapBeforeEventIds: [] }; +const EMPTY: TimelineState = { events: [], pendingRootIds: [], gapBeforeEventIds: [], echoKey: "" }; const THREAD_EMPTY: ThreadPreviewState = { events: [], totalCount: 0 }; const THREAD_FULL_EMPTY: ThreadFullState = { root: undefined, @@ -220,9 +226,15 @@ function snapshot(roomId: string): TimelineState { } } + const echoKey = events + .filter((ev) => ev.status) + .map((ev) => `${ev.getId()}:${ev.status}`) + .join(","); + const cached = snapshotCache.get(room); if ( cached && + cached.echoKey === echoKey && cached.events.length === events.length && cached.events[events.length - 1] === events[events.length - 1] && cached.pendingRootIds.length === pendingRootIds.length && @@ -232,7 +244,7 @@ function snapshot(roomId: string): TimelineState { ) { return cached; } - const next = { events, pendingRootIds, gapBeforeEventIds }; + const next = { events, pendingRootIds, gapBeforeEventIds, echoKey }; snapshotCache.set(room, next); return next; } @@ -337,12 +349,18 @@ export function makeSubscribe(roomId: string) { const onTimelineReset = (room?: Room) => { if (room?.roomId === roomId) cb(); }; + // Fires when a local echo's status changes or it is cancelled or + // resent; the event is mutated in place so no Timeline event follows. + const onLocalEcho = (_ev: MatrixEvent, room: Room) => { + if (room.roomId === roomId) cb(); + }; client.on(RoomEvent.Timeline, onTimeline); client.on(RoomEvent.TimelineReset, onTimelineReset); client.on(ClientEvent.Room, onRoom); const room = client.getRoom(roomId); room?.on(RoomEvent.Timeline, onTimeline); room?.on(RoomEvent.TimelineReset, onTimelineReset); + room?.on(RoomEvent.LocalEchoUpdated, onLocalEcho); fetchSubscribers.add(cb); const unsubPeg = MatrixClientPeg.subscribe(cb); return () => { @@ -351,6 +369,7 @@ export function makeSubscribe(roomId: string) { client.off(ClientEvent.Room, onRoom); room?.off(RoomEvent.Timeline, onTimeline); room?.off(RoomEvent.TimelineReset, onTimelineReset); + room?.off(RoomEvent.LocalEchoUpdated, onLocalEcho); fetchSubscribers.delete(cb); unsubPeg(); };