Exceptions being thrown instead handling errors in result pattern

[igorquintaes]

Hello. First of all, thanks for writing this awesome library. I am surely going to use it in my next project.

Today I faced an exception while querying GET /upload (swagger ref). When there is none pending upload, the endpoint returns a 404 Status Code with a description inside the body response. But even as a error statuscode, it can be a valid scenario when we need to check pending uploads to continue/abandon.

Sample code:

var result = await mangaDex.User.Login(username, password);
var token = token = result.Data.Session;
var uploadResponse = await mangaDex.Upload.Get(token);

HttpRequestException: Response status code does not indicate success: 404 (Not Found).

   em System.Net.Http.HttpResponseMessage.EnsureSuccessStatusCode()
   em CardboardBox.Http.HttpBuilder.<Json>d__32`1.MoveNext()
   em CardboardBox.Http.HttpBuilder.<Result>d__29`1.MoveNext()
   em MangaDexSharp.MangaDexUploadService.<Get>d__4.MoveNext()
   em DreamBot.App.Services.MangaDexService.<ClearPendingUploads>d__7.MoveNext() em C:\Users\Striter\source\repos\DreamBot\DreamBot.App\Services\MangaDexService.cs:linha 51
   em DreamBot.App.Commands.DebugCommands.<MDex>d__8.MoveNext() em C:\Users\Striter\source\repos\DreamBot\DreamBot.App\Commands\DebugCommands.cs:linha 59

I have been expecting to handle error in response, not using try/catch (because 404 is ). Something like that:

// Handle 404 scenario
if (uploadResponse.Errors.Any(x => x.Status == StatusCodes.Status404NotFound))
    return Result.Ok();

// Handle other error statuscodes (thinking in result pattern to avoid try/catchs)
if (uploadResponse.Errors.Any())
    return Result.Fail(uploadResponse.Errors.Select(x => new Error($"{x.Status} - {x.Title} - {x.Detail}")));
    
// More code bellow to success status code scenario

Did I mistaken in something?
Thanks in advance!

[calico-crusade]

Ah. Yeah. This is an issue with the library I'm using for making the HTTP requests CardboardBox.Http.
I've committed a fix, I'm just waiting for the changes to propagate to Nuget, and once that happens, I'm going to add an option to this library to allow to toggle this behavior on and off (as implementing it will break any current use of the library).

This will come in the form of a configuration option throwOnError:

//On individual APIs:
var api = MangaDex.Create(throwOnError: true);
//On DI services:
services.AddMangaDex(throwOnError: true);
//Via configuration:
{
  "MangaDex": {
    "ThrowOnError": true
  }
}

I'll update this issue once said change has been made, and nuget has indexed the update.

Edit:
I should probably clarify, this change will make it so the application will no longer throw exceptions when an error status code (something other than 2xx) is returned by the API. Instead, downstream applications will need to handle error checks themselves.
The configuration option will toggle the old logic on instead; setting throwOnError to true will throw an exception when an error status code is returned (how the application used to work).

There is also a new property on all MD return root objects called ErrorOccurred. This is a shorthand for result == "error".

[igorquintaes]

Got it, I will be waiting the change. As you already may know, the currently exception throwing make error investigation hard to be done (like 400 - bad requests), since we haven't response body to check what is wrong in our payload. Handling errors inside our own applications, as you intend to release, should fit perfectly. Thanks!

[calico-crusade]

The change has been committed. Just waiting on Nuget to index the change and then you can update to version 1.0.18 and it should start returning the errors to you instead of throwing exceptions.

If you want it to start throwing exceptions again, you'll need to set throwOnError to true in the configuration.
Let me know if this fixes your issue. If I does, I'll close this issue (or you can close it, if you want).

[igorquintaes]

I will be able to check in next few hours.

[igorquintaes]

I just checked it and stopped to throw exceptions. Thank you, I will close this issue.

Thanks for your time and fast fixing!

I am also receiving an error while trying to upload a file, and there is no info inside the returned object. Looks like a failure when the lib tries to parse the json of a bad request error response. I only can analyze it better later. If I notice that makes sense i am going to open a new issue, okay? Right now I am watching the final match of a soccer cup here in my country 😂

edit: does not look related to mangadex-sharp. I downloaded it and cardboardbox.http source and, while debugging, found an 400 response with html body:

<html>\r\n<head><title>400 Bad Request</title></head>\r\n<body>\r\n<center><h1>400 Bad Request</h1></center>\r\n<hr><center>nginx</center>\r\n</body>\r\n</html>\r\n

I will try to contact mangadex. Thanks again!