From e6de3598724194e7d126ed2beaa32bbb153cafce Mon Sep 17 00:00:00 2001 From: Marcello Costagliola Date: Mon, 5 Oct 2026 04:12:44 +0200 Subject: [PATCH] Post a message whose attachment can't be previewed instead of failing The thumbnail or video preview is generated while the message is posted. When ffmpeg or libvips can't decode the file (a truncated upload, an .mp4 holding only audio), the error escaped after the message had been saved: the request failed with a 500, the message was never broadcast, and the sender's upload stayed at 100% while the rest of the files in that drop were never sent. Posting the message without a preview keeps the file and lets everyone see it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JVFo3Lt9T8M5NR7KxVvsZ2 --- app/models/message/attachment.rb | 3 +++ test/controllers/messages_controller_test.rb | 12 +++++++++++ test/models/message/attachment_test.rb | 21 ++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/app/models/message/attachment.rb b/app/models/message/attachment.rb index bc727ba..4b895ea 100644 --- a/app/models/message/attachment.rb +++ b/app/models/message/attachment.rb @@ -30,6 +30,7 @@ module Message::Attachment attachment&.analyze end + # A file that ffmpeg or libvips can't decode is still the message: post it without a preview. def process_attachment_thumbnail case when attachment.video? @@ -37,5 +38,7 @@ module Message::Attachment when attachment.representable? attachment.representation(:thumb).processed end + rescue ActiveStorage::PreviewError, Vips::Error => error + Rails.logger.warn "Posted #{attachment.filename} without a preview: #{error.class}: #{error.message.lines.first&.chomp}" end end diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index 8088694..e5bfc14 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -57,6 +57,18 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest end end + test "creating a message with an image that can't be decoded broadcasts the message to the room" do + webp = Vips::Image.new_from_file(file_fixture("moon.jpg").to_s).webpsave_buffer + broken = Rack::Test::UploadedFile.new(StringIO.new(webp.byteslice(0, webp.bytesize / 2)), "image/webp", original_filename: "broken.webp") + + post room_messages_url(@room, format: :turbo_stream), params: { message: { attachment: broken, client_message_id: 999 } } + + assert_response :success + assert_rendered_turbo_stream_broadcast @room, :messages, action: "append", target: [ @room, :messages ] do + assert_select ".message__body a[href*='broken.webp']" + end + end + test "creating a message broadcasts unread room to each member" do @room.users.each do |member| assert_broadcasts UnreadRoomsChannel.stream_name_for(member.id), 1 do diff --git a/test/models/message/attachment_test.rb b/test/models/message/attachment_test.rb index 47d71c1..6925eb0 100644 --- a/test/models/message/attachment_test.rb +++ b/test/models/message/attachment_test.rb @@ -19,6 +19,20 @@ class Message::AttachmentTest < ActiveSupport::TestCase assert_equal message.plain_text_body, "moon.jpg" end + test "creating a message keeps an image that can't be decoded" do + webp = Vips::Image.new_from_file(file_fixture("moon.jpg").to_s).webpsave_buffer + message = create_unreadable_attachment_message(webp.byteslice(0, webp.bytesize / 2), "broken.webp") + + assert_equal "broken.webp", message.reload.attachment.filename.to_s + assert_nil message.attachment.representation(:thumb).image + end + + test "creating a message keeps a video that can't be decoded" do + message = create_unreadable_attachment_message(file_fixture("alpha-centuri.mov").binread(64), "broken.mov") + + assert_equal "broken.mov", message.reload.attachment.filename.to_s + assert_not message.attachment.preview(format: :webp).image.attached? + end private def create_attachment_message(file, content_type) @@ -27,4 +41,11 @@ class Message::AttachmentTest < ActiveSupport::TestCase client_message_id: "message", attachment: fixture_file_upload(file, content_type) end + + def create_unreadable_attachment_message(content, filename) + rooms(:hq).messages.create_with_attachment! \ + creator: users(:david), + client_message_id: "message", + attachment: { io: StringIO.new(content), filename: filename } + end end