Error handling is still broken with v6.0.3

rhegner commented 4 months ago

Expected Behavior

When calling functions like StatObjectAsync or GetObjectAsync, the SDK should throw exceptions if connection to Minio could not be established.

Current Behavior

No exceptions are thrown in SDK v6.0.2 and v6.0.3.

This is especially bad in this example where we rely on proper exception handling to detect if an object exists:

        private static async Task<bool> Exists(IMinioClient minioClient, string bucket, string objectName)
        {
            try
            {
                var request = new StatObjectArgs()
                    .WithBucket(bucket)
                    .WithObject(objectName);
                await minioClient.StatObjectAsync(request);
                return true;
            }
            catch (ObjectNotFoundException)
            {
                return false;
            }
        }

The code returns true even though the Minio server could not even be reached.

Steps to Reproduce (for bugs)

This is my testbed:

using Microsoft.Extensions.Configuration;
using Minio;
using Minio.DataModel.Args;
using Minio.Exceptions;

namespace MinioSdkTest
{
    internal class TestException : Exception;

    internal class Program
    {
        private const string ValidMinioEndpoint = "http://localhost:9000";
        private const string InvalidMinioEndpoint = "http://localhost:9005";
        private const string Bucket = "miniotest";
        private const string ExistingObject = "Rechnung (34).pdf";
        private const string NonexistingObject = "Rechnung (xx).pdf";

        static async Task Main(string[] args)
        {
            // Minio Version                                6.0.0   6.0.1   6.0.2   6.0.3
            // --------------------------------------------------------------------------
            await TestExistsWithExistingObject();       //  ok      ok      ok      ok
            await TestExistsWithNonexistingObject();    //  ok      NOK     ok      ok
            await TestExistsFromInvalidEndpoint();      //  ok      ok      NOK     NOK
            await TestDownloadWithExistingObject();     //  ok      ok      ok      ok
            await TestDownloadWithNonexistingObject();  //  ok      ok      ok      ok
            await TestDownloadFromInvalidEndpoint();    //  ok      ok      NOK     NOK
        }

        private static async Task TestExistsWithExistingObject()
        {
            var minioClient = GetMinioClient(ValidMinioEndpoint);
            var exists = await Exists(minioClient, Bucket, ExistingObject);
            if (!exists)
                throw new TestException();
        }

        private static async Task TestExistsWithNonexistingObject()
        {
            var minioClient = GetMinioClient(ValidMinioEndpoint);
            var exists = await Exists(minioClient, Bucket, NonexistingObject);
            if (exists)
                throw new TestException();
        }

        private static async Task TestExistsFromInvalidEndpoint()
        {
            try
            {
                var minioClient = GetMinioClient(InvalidMinioEndpoint);
                var exists = await Exists(minioClient, Bucket, NonexistingObject);
                throw new TestException();  // we should never get here
            }
            catch (TestException)
            {
                throw;
            }
            catch (ConnectionException)
            {
                // this is the expected outcome (throw exception if trying to access invalid endpoint)
            }
        }

        private static async Task TestDownloadWithExistingObject()
        {
            var minioClient = GetMinioClient(ValidMinioEndpoint);
            var data = await Download(minioClient, Bucket, ExistingObject);
            if (data.Length == 0)
                throw new TestException();
        }

        private static async Task TestDownloadWithNonexistingObject()
        {
            try
            {
                var minioClient = GetMinioClient(ValidMinioEndpoint);
                var data = await Download(minioClient, Bucket, NonexistingObject);
                throw new TestException();  // we should never get here
            }
            catch (TestException)
            {
                throw;
            }
            catch (ObjectNotFoundException)
            {
                // this is the expected outcome (throw exception if trying to access nonexisting object)
            }
        }

        private static async Task TestDownloadFromInvalidEndpoint()
        {
            try
            {
                var minioClient = GetMinioClient(InvalidMinioEndpoint);
                var data = await Download(minioClient, Bucket, NonexistingObject);
                throw new TestException();  // we should never get here
            }
            catch (TestException)
            {
                throw;
            }
            catch (ConnectionException)
            {
                // this is the expected outcome (throw exception if trying to access invalid endpoint)
            }
        }

        private static async Task<bool> Exists(IMinioClient minioClient, string bucket, string objectName)
        {
            try
            {
                var request = new StatObjectArgs()
                    .WithBucket(bucket)
                    .WithObject(objectName);
                await minioClient.StatObjectAsync(request);
                return true;
            }
            catch (ObjectNotFoundException)
            {
                return false;
            }
        }

        private static async Task<Stream> Download(IMinioClient minioClient, string bucket, string objectName)
        {
            var stream = new MemoryStream();
            try
            {
                var request = new GetObjectArgs()
                    .WithBucket(bucket)
                    .WithObject(objectName)
                    .WithCallbackStream((s, ct) => s.CopyToAsync(stream, ct));
                await minioClient.GetObjectAsync(request);
                stream.Position = 0;
                return stream;
            }
            catch
            {
                stream.Dispose();
                throw;
            }
        }

        private static IMinioClient GetMinioClient(string endpoint)
        {
            var configuration = new ConfigurationBuilder().AddUserSecrets<Program>().Build();
            var accessKey = configuration["minio_access_key"];
            var secretKey = configuration["minio_secret_key"];

            var endpointUri = new Uri(endpoint);
            var minioClient = new MinioClient()
                .WithEndpoint(endpointUri.Authority)   // Authority is host:port
                .WithCredentials(accessKey, secretKey);
            if (endpointUri.Scheme == "https")
                minioClient.WithSSL();
            minioClient.Build();
            return minioClient;
        }
    }
}

Regression

Error handling is broken (in different ways) since v6.0.1.

Your Environment

Minio 6.0.3