Skip to content

Stop a failed media send from being posted twice #290

Description

@HMarzban

Problem

When a plain send with attachments fails, its media stays in two places. The failed row holds it with Retry, and the composer strip holds it too. A plain send has no reply, comment, or edit. An attachment is the composer entry, and its upload is the file in the media bucket.

On 14ab7f9c1. Paths are under apps/webapp/src/components/chatroom/.

  1. A send that fits one row clears the composer before dispatch, unless it is an edit (utils/outboundMessagePipeline.ts:52-55, :125, apps/webapp/src/utils/chunkHtmlContent.ts:60-68). Attachments on a longer message are refused (utils/outboundMessagePipeline.ts:92-97). cleanupAfterSubmit() clears the text, the saved draft, and the reply and comment memory (components/MessageComposer/hooks/useComposerSubmit.ts:86-116, :160). It does not clear the attachments.
  2. useSendMessage.send appends a pending row with the text and the media, then inserts it (hooks/useSendMessage.ts:95-116). A row with media shows in every feed mode (utils/channelFeedProjection.ts:26-27). On an error, the row turns failed (hooks/useSendMessage.ts:130-139). Both feed bodies then show Failed to send · Retry · Delete under it (components/ChatList/DesktopMessageBody.tsx:88, components/ChatList/MobileMessageBody.tsx:94).
  3. dispatchOutboundChunk throws on the failed result (utils/outboundMessagePipeline.ts:282-286). The submit catch shows a toast and returns (components/MessageComposer/hooks/useComposerSubmit.ts:172-180). So it never reaches clearAttachments({ deleteStorage: false }) at :182. The strip keeps the same ready attachments.

What can then go wrong:

  • Two copies. Send stays enabled, because ready attachments pass the send gate with no text (utils/composerSendGate.ts:24-27). A new send gets a new client id (hooks/useSendMessage.ts:95). Retry sends the failed row again under its own id (hooks/useSendMessage.ts:144-172). The server accepts a path that another message already uses. validate_message_medias() checks the owner prefix, the file type, and that the upload exists (packages/supabase/scripts/10-3-func-message.sql:438-506). If the user does both, the channel gets two messages with the same media.
  • A stale strip after Retry. When Retry succeeds, the strip still holds the attachments. The next message the user sends carries the same media again.
  • A lost caption. The early clear removed the caption. A new send from the strip posts the media without it.
  • Shared uploads. Both copies point at the same uploads. Delete on the failed row removes them from the bucket (components/MessageCard/components/MessageFailedRow.tsx:25-31). The owner may delete own uploads (packages/supabase/scripts/12-buckets.sql:197-201). Deleting a sent copy also removes its uploads, through the soft-delete trigger (packages/supabase/scripts/10-3-func-message.sql:92-93, :120). The other copy then shows "Image unavailable" (components/MessageCard/components/MessageContent/components/MessageMediaImageLink.tsx:53-61).
  • Each side breaks the other. Removing a tile from the strip deletes its upload (components/MessageComposer/hooks/useComposerAttachments.ts:64-75, utils/chatMediaUploadRunner.ts:119-131). Retry then leaves the row failed, because the upload is gone (utils/sendChatMessage.ts:54-61, :107-109). If the user deletes the failed row first, Send from the strip fails its storage check. The toast says "Attachments are still uploading. Wait a moment and try again." (components/MessageComposer/hooks/useComposerSubmit.ts:143-146).

The text of a failed send has one owner: the failed row, which holds it for Retry. The media has two owners: the failed row and the strip.

The Cypress spec keeps attachment strip after failed send asserts that the strip keeps the file (apps/webapp/cypress/e2e/chatroom/attachments.cy.ts:313-332). A fix changes that spec.

Steps to reproduce

  1. Open a channel. Do not start a reply, a comment, or an edit.
  2. Attach one image and type a short caption. Wait for the upload to finish.
  3. In the browser DevTools, block the request URL rest/v1/messages. Press Send.
  4. See the failed row with the image and Retry. See the same image still in the composer strip.
  5. Unblock the URL. Press Send in the composer, then press Retry on the failed row.
  6. See two messages with the same image. Only the one from Retry has the caption.
  7. Delete one of them, then reload the page. See "Image unavailable" on the other.

Acceptance criteria

  • After a plain send with attachments ends in a failed row, the strip no longer holds the sent attachments. The failed row shows the media, with Retry and Delete.
  • Retry on that row then posts one message with the media. No second message with the same media appears.
  • Removing the sent attachments from the strip after the failure deletes no upload, so Retry can succeed.
  • Delete on the failed row still removes the row and deletes its uploads.
  • When the storage check fails before the send, the toast shows, no row appears, and the strip keeps its attachments, as today.
  • A successful plain send with attachments still removes the sent attachments from the strip and keeps their uploads.
  • The Cypress spec for a failed media send asserts an empty strip and a failed row.

Agent Brief

Category: bug
Summary: After a plain send with attachments fails, the failed row must be the only owner of its media. The composer strip must not keep a second copy.

Current behavior:
A plain send clears the text before dispatch, and the pending row holds the text and the media. When the insert fails, the submit catch returns before the strip is cleared. The strip and the failed row then hold the same uploads. Send and Retry can each post them, and removing either side breaks the other.

Desired behavior:
Once the feed row holds the media, the strip drops the sent attachments without deleting their uploads. From then on, Retry and Delete on the failed row own the media. A send that fails before any row appears keeps the strip as today.

Key interfaces:

  • useComposerSubmit()submitMessage, its catch, and cleanupAfterSubmit().
  • clearAttachments({ deleteStorage }) from useComposerAttachments().
  • removeAttachment(id) — removes one tile and deletes its upload. A clear after a send must not use it.
  • dispatchOutboundChunk() — throws when the context send returns failed.
  • SendResult from useSendMessage().sendsent and failed come after the pending row is offered to the feed. A row with media always passes the feed filter. auth_required means no row.
  • MessageFailedRow — the Retry and Delete controls under a failed row. Delete removes the uploads.

Out of scope

Notes

Coordinate with #263. It builds the send hand-off on the same early clear, and its failed state keeps Retry and Delete.

Coordinate with #266. Both fixes change what happens to strip attachments when a send leaves the composer.

If the #266 fix keeps a reply's attachments in the strip, a failed reply has the same two owners. Apply the same rule to replies then.

The chatroom rules name the failed row as the retry path (apps/webapp/src/components/chatroom/CLAUDE.md:21).

The success path clears the whole strip (apps/webapp/src/components/chatroom/components/MessageComposer/hooks/useComposerAttachments.ts:77-86). So a file added while a send is still running is dropped too. A fix that removes only the sent attachments avoids that on both paths.

After this fix, the failed row is the only copy of the media. That row lives only in the message list data. A jump that loads a new window drops it (hooks/useJumpTo.ts:45, hooks/useScrollToMessage.ts:44), and so do a reload and a channel switch. Today the strip still holds the media after a jump. The failed text already has this limit, and the fix gives the media the same limit.

Present since 7b6622687 (2026-06-24). That commit added chat media, the strip clear, now at apps/webapp/src/components/chatroom/components/MessageComposer/hooks/useComposerSubmit.ts:182, and the spec that keeps the strip. The failed row with Retry and Delete already existed.

The evidence is a code trace on 14ab7f9c1, confirmed by two verifiers. The Cypress spec above asserts the strip half, but no CI job runs it (#276). It was not reproduced in a browser.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ChatRelated to chat featuresbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions