From aae56c35290da2a6251b773b3f93845e21791823 Mon Sep 17 00:00:00 2001 From: Cameron Bothner Date: Thu, 23 Aug 2018 23:36:43 -0400 Subject: [PATCH] Handle only specifically relevant Azure HTTPErrors The Azure gem uses `Azure::Core::Http::HTTPError` for everything: checksum mismatch, missing object, network unavailable, and many more. (https://www.rubydoc.info/github/yaxia/azure-storage-ruby/Azure/Core/Http/HTTPError). Rescuing that class obscures all sorts of configuration errors. We should check the type of error in those rescue blocks, and reraise when needed. --- activestorage/CHANGELOG.md | 7 +++++++ .../active_storage/service/azure_storage_service.rb | 12 +++++++----- 2 files changed, 14 insertions(+), 5 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..8de3889cb5 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 @@ def initialize(storage_account_name:, storage_access_key:, container:) 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 @@ def delete(key) 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 @@ -155,8 +154,11 @@ def stream(key) 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