BucketExistsAsync seems to need a bit of refactoring - Githubissues

minio / minio-dotnet

MinIO Client SDK for .NET

https://docs.min.io/docs/dotnet-client-quickstart-guide.html

Apache License 2.0

575 stars 230 forks source link

BucketExistsAsync seems to need a bit of refactoring #1132

Open marwa-elfakir-logi opened 4 months ago

marwa-elfakir-logi commented 4 months ago

As of version 6.0.3 today this is current implementation of BucketExistsAsync

I noticed that in the part

if an InternalClientException is caught, then the function will return true in these 2 cases :

ice.ServerResponse is null && ice.ServerResponse is not null
ice.ServerResponse is not null && HttpStatusCode.NotFound != ice.ServerResponse.StatusCode

The first case is a contradictory condition maybe u meant to write second case only ? or maybe smt else (either way it seems to need some refactoring)
The second case is a bit confusing ? why would we consider the bucket exists if the StatusCode was different from NotFound ? Do all other InternalClientException.ServerResponse.StatusCode cases mean the bucket exist ?? shouldn't we let the exception be thrown in some cases ?

ebozduman commented 4 months ago

@marwa-elfakir-logi ,

It looks like this is the same issue with #1131 you filed before. Although they are duplicates, I'll keep it open so that you can try all your scenarios to test the fix, PR #1141, for this issue and if you are satisfied, to close the issue.