minio / minio-dotnet

MinIO Client SDK for .NET
https://docs.min.io/docs/dotnet-client-quickstart-guide.html
Apache License 2.0
573 stars 229 forks source link

Support using PutObjectAsync return a HttpStatusCode and throw Exception when StatusCode not equal 200 #1171

Open lengochiep99 opened 2 months ago

lengochiep99 commented 2 months ago

PutObjectAsync return a PutObjectResponse, but property HttpStatusCode and ResponseContent is a internal. I checked log, and HttpStatusCode is 400, but cann't access the attribute ResponseStatusCode

ArminShoeibi commented 2 months ago

Yes, we need to change these properties to public

https://github.com/minio/minio-dotnet/blob/7440578bb43331518123431a6eef908760d23237/Minio/DataModel/Response/GenericResponse.cs#L29

https://github.com/minio/minio-dotnet/blob/7440578bb43331518123431a6eef908760d23237/Minio/DataModel/Response/GenericResponse.cs#L30

n-goncalves commented 2 weeks ago

Yes, we need to change these properties to public

https://github.com/minio/minio-dotnet/blob/7440578bb43331518123431a6eef908760d23237/Minio/DataModel/Response/GenericResponse.cs#L29

https://github.com/minio/minio-dotnet/blob/7440578bb43331518123431a6eef908760d23237/Minio/DataModel/Response/GenericResponse.cs#L30

It's a bit more than that.

Currently the documentation and the samples that are available here or in https://min.io/docs/minio/linux/developers/dotnet/quickstart.html makes one assume that an upload is successful if no exception is triggered. This is wrong and dangerous since that assumption can make you lose files as I already wrote in #1176. As the person who opened this issue, we've also seen PutObjectResponse with a non 200 HttpStatusCode, 400 or 500 to be precise.

Example of PutObject documentation taken from README:

try 
{
...
// Upload a file to bucket.
 var putObjectArgs = new PutObjectArgs()
  .WithBucket(bucketName)
  .WithObject(objectName)
 .WithFileName(filePath)
 .WithContentType(contentType);
await minio.PutObjectAsync(putObjectArgs).ConfigureAwait(false);
Console.WriteLine("Successfully uploaded " + objectName );
} 
...
ArminShoeibi commented 1 week ago

Yes, we need to change these properties to public https://github.com/minio/minio-dotnet/blob/7440578bb43331518123431a6eef908760d23237/Minio/DataModel/Response/GenericResponse.cs#L29

https://github.com/minio/minio-dotnet/blob/7440578bb43331518123431a6eef908760d23237/Minio/DataModel/Response/GenericResponse.cs#L30

It's a bit more than that.

Currently the documentation and the samples that are available here or in https://min.io/docs/minio/linux/developers/dotnet/quickstart.html makes one assume that an upload is successful if no exception is triggered. This is wrong and dangerous since that assumption can make you lose files as I already wrote in #1176. As the person who opened this issue, we've also seen PutObjectResponse with a non 200 HttpStatusCode, 400 or 500 to be precise.

Example of PutObject documentation taken from README:

try 
{
...
// Upload a file to bucket.
 var putObjectArgs = new PutObjectArgs()
  .WithBucket(bucketName)
  .WithObject(objectName)
 .WithFileName(filePath)
 .WithContentType(contentType);
await minio.PutObjectAsync(putObjectArgs).ConfigureAwait(false);
Console.WriteLine("Successfully uploaded " + objectName );
} 
...

I'm sorry, I didn't get that, Why is it more than publicizing those modifiers?

This merge request is ready and makes those modifiers public https://github.com/minio/minio-dotnet/pull/1195

n-goncalves commented 1 week ago

Publicizing those modifiers isn't the same as triggering an Exception if the status code isn't 200 which is the title of this issue.

Is it expected that PutObjectAsync doesn't trigger an Exception but still fails to do the upload ? This is mentioned nowhere in the MinIO .NET Client API Documentation. The only way to know that is to read the code in depth ... or experience it the hard way and lose a file due to this.