ExecuteLoginAsync throwing FlurlHttpException instead of SLException

[EricWu91]

At https://github.com/bgmulinari/B1SLayer/blob/f6f2d6e36c68f0b6a9a39b405b7ed7856cd28d04/B1SLayer/SLConnection.cs#L356C11-L365C14 :

 catch (FlurlHttpException ex)
 {
    try
    {
        if (ex.Call.HttpResponseMessage == null) throw;
        var response = await ex.GetResponseJsonAsync<SLResponseError>();
        throw new SLException(response.Error.Message.Value, response.Error, ex);
     }
     catch { throw ex; }
}

If a FlurlHttpException is thrown (e.g. when a 401 is returned due to invalid provided credentials), I see you throw a new SLException with the error data deserialized and parsed. However, the next catch block throws the original FlurlHttpException, which overrides that.

Is that supposed to happen?

[bgmulinari]

Hi, @EricWu91.

This is indeed an oversight on my part. The throw at the bottom should only be executed if there was a problem and the SLException couldn't be thrown.

I'll be fixing this in the next release, likely 2.0.0. Thanks for reporting!

[EricWu91]

Would you like me to create a PR and correct it in this release?

[EricWu91]

Did it nonetheless :) If you think that's not what you want, I can correct it.

https://github.com/bgmulinari/B1SLayer/pull/71

[bgmulinari]

Thanks for the contribution, @EricWu91. I'll be releasing a new version before 2.0.0 with your fix.

If you could just change your PR to the dev branch, I would appreciate it.

[EricWu91]

Done. I wrote it in the simplest way I could think of to accomplish that. If it doesn't fit the way you envisioned it, let me know and I'll rewrite it.

[bgmulinari]

@EricWu91, I was able to deliver a prerelease of version 2.0.0 which already contains your fix, so I think I'll leave version 1.3.x as is, as I don't see this bug as something too impactful.

If you are able to test it and report back, I would really appreciate it.

Download it here: https://www.nuget.org/packages/B1SLayer/2.0.0-pre1

Thanks again for your contribution.

[EricWu91]

Thanks, it worked.

Can you point me to a ref describing the changes from 1.32 to 2.0? Are there breaking changes?

[bgmulinari]

The main change is the upgrade from Flurl 3 to Flurl 4, which drops Newtonsoft.Json in favor of System.Text.Json. Therefore, there are a few breaking changes in regards to JSON configuration and the removal of dynamic-returning methods.

Keep in mind this is the first prerelease of 2.0.0, there might be other releases with further changes and improvements depending on feedback. I'll have the complete release notes detailing all changes when the final version is out.

[EricWu91]

Ok! Thanks for your response. I believe we can close this issue now.