Merge pull request #33666 from cbothner/fail-gracefully-from-activestorage-file-not-found

Fail more gracefully from ActiveStorage missing file exceptions

2 files changed, 29 insertions, 0 deletions

diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb
index c053052f6f..4bc61d13f3 100644
--- a/activestorage/test/controllers/disk_controller_test.rb
+++ b/activestorage/test/controllers/disk_controller_test.rb
@@ -31,6 +31,14 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
     assert_equal " worl", response.body
   end
 
+  test "showing blob that does not exist" do
+    blob = create_blob
+    blob.delete
+
+    get blob.service_url
+    assert_response :not_found
+  end
+
 
   test "directly uploading blob with integrity" do
     data = "Something else entirely!"
diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb
index 30cfca4e36..58f189af2b 100644
--- a/activestorage/test/service/shared_service_tests.rb
+++ b/activestorage/test/service/shared_service_tests.rb
@@ -50,6 +50,13 @@ module ActiveStorage::Service::SharedServiceTests
       assert_equal FIXTURE_DATA, @service.download(@key)
     end
 
+    test "downloading a nonexistent file" do
+      assert_raises(ActiveStorage::FileNotFoundError) do
+        @service.download(SecureRandom.base58(24))
+      end
+    end
+
+
     test "downloading in chunks" do
       key = SecureRandom.base58(24)
       expected_chunks = [ "a" * 5.megabytes, "b" ]
@@ -68,11 +75,25 @@ module ActiveStorage::Service::SharedServiceTests
       end
     end
 
+    test "downloading a nonexistent file in chunks" do
+      assert_raises(ActiveStorage::FileNotFoundError) do
+        @service.download(SecureRandom.base58(24)) {}
+      end
+    end
+
+
     test "downloading partially" do
       assert_equal "\x10\x00\x00", @service.download_chunk(@key, 19..21)
       assert_equal "\x10\x00\x00", @service.download_chunk(@key, 19...22)
     end
 
+    test "partially downloading a nonexistent file" do
+      assert_raises(ActiveStorage::FileNotFoundError) do
+        @service.download_chunk(SecureRandom.base58(24), 19..21)
+      end
+    end
+
+
     test "existing" do
       assert @service.exist?(@key)
       assert_not @service.exist?(@key + "nonsense")