Merge pull request #33667 from cbothner/azure-service-swallowing-all-errors

Handle only specifically relevant Azure HTTPErrors in ActiveStorage::Service::AzureStorageService

2 files changed, 20 insertions, 7 deletions

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index b592f79ca6..92e300a440 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   `ActiveStorage::Service::AzureStorageService` only handles specifically
+    relevant types of `Azure::Core::Http::HTTPError`. It previously obscured
+    other types of `HTTPError`, which is the azure-storage gem’s catch-all
+    exception class.
+
+    *Cameron Bothner*
+
 *   `ActiveStorage::DiskController#show` generates a 404 Not Found response when
     the requested file is missing from the disk service. It previously raised
     `Errno::ENOENT`.
diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb
index 66aabc1f9f..4fde35bdeb 100644
--- a/activestorage/lib/active_storage/service/azure_storage_service.rb
+++ b/activestorage/lib/active_storage/service/azure_storage_service.rb
@@ -19,10 +19,8 @@ module ActiveStorage
 
     def upload(key, io, checksum: nil)
       instrument :upload, key: key, checksum: checksum do
-        begin
+        handle_errors do
           blobs.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum)
-        rescue Azure::Core::Http::HTTPError
-          raise ActiveStorage::IntegrityError
         end
       end
     end
@@ -55,7 +53,8 @@ module ActiveStorage
       instrument :delete, key: key do
         begin
           blobs.delete_blob(container, key)
-        rescue Azure::Core::Http::HTTPError
+        rescue Azure::Core::Http::HTTPError => e
+          raise unless e.type == "BlobNotFound"
           # Ignore files already deleted
         end
       end
@@ -128,8 +127,12 @@ module ActiveStorage
 
       def blob_for(key)
         blobs.get_blob_properties(container, key)
-      rescue Azure::Core::Http::HTTPError
-        false
+      rescue Azure::Core::Http::HTTPError => e
+        if e.type == "BlobNotFound"
+          false
+        else
+          raise
+        end
       end
 
       def format_expiry(expires_in)
@@ -155,8 +158,11 @@ module ActiveStorage
       def handle_errors
         yield
       rescue Azure::Core::Http::HTTPError => e
-        if e.type == "BlobNotFound"
+        case e.type
+        when "BlobNotFound"
           raise ActiveStorage::FileNotFoundError
+        when "Md5Mismatch"
+          raise ActiveStorage::IntegrityError
         else
           raise
         end