minio-dotnet
minio-dotnet copied to clipboard
minio
Error handling is still broken with v6.0.3
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
PutObjectAsync and BucketExistsAsync also seem to ignore errors in 6.0.3 (6.0.1 is fine)
I think PR #1141 has fixed several issues including this one. I appreciate if you could test and verify the fix. Please let me know about your test results and findings.
@ebozduman I did not run my tests yet, but just from looking at the code I still have some concerns regarding error handling, see https://github.com/minio/minio-dotnet/pull/1141#issuecomment-2250123299
@rhegner
I agree the error handling is complex and problematic and its maintenance and testing is cumbersome. Until it is redesigned though, we have to support it IMHO, or until mngmt makes a decision about it.
I appreciate it if you could test the fix in your environment and let us know an of course open up a new issue if required.
@tinohager
There is some work that has been done and completed to fix the issue, but testing the fix and tuning it up for all possible scenarios requires considerable amount of time. Unfortunately, sometimes a higher priority task might slow down the progress of this issue and the reviewers are expected to be extremely thorough when the fix is ready.
Maybe you should at least mark the package in Nuget/Github that there are problems with the current version.
Pin Issue
I would welcome any development on this issue at all.
Just to emphasize, anyone currently starting a new project and using the most recent version of the NuGet package will get 0 error reporting and handling if they make any mistake setting up any Minio integration.
For any unfortunate future reader: I could personally fix my issue by adding .WithSSL(false) to give me:
services.AddMinio(options =>
{
options
.WithEndpoint(config["STORAGE_ENDPOINT"])
.WithCredentials(config["STORAGE_ACCESS_KEY"], config["STORAGE_SECRET_KEY"])
.WithSSL(false)
.Build();
});
@wardboumans Thank you for confirming it. We are aware of it and unfortunately it took a long time to address it. It is reevaluated and will be addressed again and available in a month frame.
Unfortunately v6.0.4 does not resolve this:
// Minio Version 6.0.0 6.0.1 6.0.2 6.0.3 6.0.4
// ----------------------------------------------------------------------------------
await TestExistsWithExistingObject(); // ok ok ok ok ok
await TestExistsWithNonexistingObject(); // ok NOK ok ok ok
await TestExistsFromInvalidEndpoint(); // ok ok NOK NOK NOK
await TestDownloadWithExistingObject(); // ok ok ok ok ok
await TestDownloadWithNonexistingObject(); // ok ok ok ok ok
await TestDownloadFromInvalidEndpoint(); // ok ok NOK NOK NOK
await TestWithInvalidCredentials(); // ok ok NOK NOK NOK
We are still stuck with v6.0.0, as the error handling is completely broken with all newer versions!
@rhegner That is unfortunately correct. Sorry, I don't have a definitive date at this point, but it'll be addressed soon.
@ebozduman At least mark the Nuget packages that they should not be used. If you don't manage to fix the error in time. The error behavior is a critical behavior from my point of view.
Hi,
we are also having this issue with 6.0.4. Will there be a solution soon? Best regards,
Patrick
Hi, I have the same problem (with version 6.0.4) Any news about the resolution?
How can any version with this bug even be available in the nuget store? Lots of api method just outright lies (BucketExistsAsync for instance returns true despite no connection being made) . Anyone using examples from official documentation will be mislead. At least put a disclaimer on the official documentation?
This comment inside a PR talks about "marking the versions as unstable" back in February but nothing was done to this day.
Totally agree with what @Todilo said, it is really disappointing that the only way to know of this issue is to have the library not working as it's supposed to and end up here trying to find the root cause.
It's not a good outlook having error handling broken for more than 1 year and more than 6 months after being reported.
I apologize for the delay. I've asked the builds to be marked as unstable a couple of times, but it fell through the cracks and I failed to follow up. There will be a PR to address this issue in 3-4 days and if the internal reviewers agree, we'll merge it to create a new package.
@ebozduman I wanted to trigger again as 4 days have already passed 😆
In the meantime, I recommend marking the package as buggy on nuget and pinning the issue on github.
nuget.org
@tinohager
I requested the nuget package to be marked as buggy/deprecated.
A fix for this issue is pushed into the PR, #1141. Although I've tested all features before the PR is created, some build tests still failed. These failures will be addressed this week.
@ebozduman Not much has happened yet. You know that you can mark the package as faulty yourself on NuGet.
PR#1141 is ready to be reviewed but nobody had time to review the fix. I expect PR#1141 to resolve the major missing exception issue in 6.0.4 and other broken older releases since 6.0
This issue will be addressed in PR #1141.
Status of PR #1141:
Found 3 issues during dotnet regitlint run; 2 resolved, 1 is still pending (an object not having a method).
However, this does not cause any problem during dotnet build and the tests run successfully. The test, which has the problematic code, was commented out in the previous releases.
If review comments are resolved, I anticipate the release process is going to be started in a couple of days.