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

[cbothner]

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.

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

[georgeclaghorn]

Can you please rebase this against master?

[cbothner]

👍 I also went ahead and made some changes to be consistent with the merged version of #33666.

[georgeclaghorn]

Thank you!

[georgeclaghorn]

Sorry, I had to revert this because the Azure Storage service tests failed in master. It’s my fault for forgetting that those tests don’t run for PRs.

It looks like Azure::Core::Http::HTTPError#type is always "Unknown" for missing-blob exceptions, which is particularly unhelpful.

/cc two frequent committers to Azure/azure-ruby-asm-core: @katmsft @yaxia

[cbothner]

That is very strange. I did make an Azure account and ran the tests on my machine before submitting the PR… but I guess I made other changes afterwards and forgot. My apologies for the oversight.

Turning individual changes on and off:

• #upload with an invalid checksum is failing with "Md5Mismatch" without the rescue
• #delete is failing with "BlobNotFound" without the rescue
• #blob_for, which calls Azure::Storage::Blob#get_blob_properties, is where we get "Unknown"

It seems that the error types are less consistent from Azure than I had thought.

I do think it would be ideal if the Azure gem set Azure::Core::Http::HTTPError#type to BlobNotFound in get_blob_properties. But if you’re comfortable continuing to rescue indiscriminately in #blob_for (which makes the tests pass) we do get value from this change simply by checking the type in #upload and #delete. "InvalidResourceName" and "AuthenticationFailed" are then correctly surfaced, where before they presented as a confusing ActiveStorage::IntegrityError or were swallowed completely.

Let me know how you’d like to proceed and I’ll resubmit as needed.

[georgeclaghorn]

If you don’t mind, please open a new PR with the #upload and #delete changes and put “r? @georgeclaghorn” at the end of the description. Thanks!