Skip to content

Commit

Permalink
[chore] Consolidate, clean up exceptions logic (#454)
Browse files Browse the repository at this point in the history
- Improve, clean up exception logic, naming and organization
  • Loading branch information
nwithan8 committed May 2, 2023
1 parent db9928a commit 022ebcb
Show file tree
Hide file tree
Showing 19 changed files with 136 additions and 84 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@

- Handle API timeout errors more gracefully (produce proper `TimeoutError` exception with readable messages)
- Add missing `Declaration` parameter to Customs Info creation parameter set
- [CHANGED] Renamed `UnexpectedHttpError` exception type to `UnknownHttpError` to better reflect its purpose.
- [REMOVED] Removed `UnknownApiError` exception type, consolidated into `UnknownHttpError` exception type.
- [CHANGED] `ExternalApiError` no longer inherits from `ApiError` (`ApiError` reserved for EasyPost API errors only).
- [IMPROVED] All `EasyPostError` exceptions and subclasses now have a `PrettyPrint` getter that returns a human-readable string representation of the error.
- Previously, only `ApiError` exceptions had this. Now, all exceptions thrown by the library should have this.
- [IMPROVED] Logic for calculating exception type to throw based on API error
- EasyPost API failures can trigger a variety of specific exceptions, all inheriting from `ApiError`.
- Non-EasyPost API/HTTP failures will trigger an `ExternalApiError` exception.


## v4.6.0 (2023-04-18)

Expand Down
58 changes: 34 additions & 24 deletions EasyPost.Tests/ExceptionsTests/ExceptionsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using EasyPost.Exceptions;
using EasyPost.Exceptions.API;
using EasyPost.Exceptions.General;
using EasyPost.Http;
using EasyPost.Models.API;
using EasyPost.Tests._Utilities;
using EasyPost.Tests._Utilities.Attributes;
Expand Down Expand Up @@ -48,7 +47,11 @@ public async Task TestApiExceptionPrettyPrint()

// Generate a dummy HttpResponseMessage with the given status code to parse
httpStatusCode = (HttpStatusCode)Enum.Parse(typeof(HttpStatusCode), statusCode.ToString(CultureInfo.InvariantCulture));
response = new() { StatusCode = httpStatusCode, Content = new StringContent(errorMessageStringJson) };
response = new()
{
StatusCode = httpStatusCode,
Content = new StringContent(errorMessageStringJson)
};

generatedError = await ApiError.FromErrorResponse(response);

Expand All @@ -60,11 +63,22 @@ public async Task TestApiExceptionPrettyPrint()

// Now test with some error-related JSON inside the response with sub-errors
errorMessageStringJson = "{\"error\": {\"code\": \"ERROR_CODE\", \"message\": \"ERROR_MESSAGE\", \"errors\": [{\"field\": \"SUB_ERROR_FIELD\", \"message\": \"SUB_ERROR_MESSAGE\"}]}}";
List<Error> subErrors = new() { new Error { Field = "SUB_ERROR_FIELD", RawMessage = "SUB_ERROR_MESSAGE" } };
List<Error> subErrors = new()
{
new Error
{
Field = "SUB_ERROR_FIELD",
RawMessage = "SUB_ERROR_MESSAGE"
}
};

// Generate a dummy HttpResponseMessage with the given status code to parse
httpStatusCode = (HttpStatusCode)Enum.Parse(typeof(HttpStatusCode), statusCode.ToString(CultureInfo.InvariantCulture));
response = new() { StatusCode = httpStatusCode, Content = new StringContent(errorMessageStringJson) };
response = new()
{
StatusCode = httpStatusCode,
Content = new StringContent(errorMessageStringJson)
};

generatedError = await ApiError.FromErrorResponse(response);

Expand All @@ -85,11 +99,9 @@ public void TestExceptionConstructors()
const string testMessage = "This is a test message.";
const string testPropertyName = "test_property";
Type testType = typeof(ExceptionsTests);
object testObj = new EasyPostError("unimportant_string");

// Test the base constructor
EasyPostError defaultError = new(testMessage);
Assert.Equal(testMessage, defaultError.Message);
// Test the base EasyPostError constructor
// Class is abstract, cannot be directly constructed

// Test the base ApiError constructor
// Constructor is protected (black-boxed), so we can't access it
Expand Down Expand Up @@ -142,12 +154,9 @@ public void TestExceptionConstructors()
UnauthorizedError unauthorizedError = new(testMessage, 0);
Assert.Equal(testMessage, unauthorizedError.Message);

UnexpectedHttpError unexpectedHttpError = new(testMessage, 0);
UnknownHttpError unexpectedHttpError = new(testMessage, 0);
Assert.Equal(testMessage, unexpectedHttpError.Message);

UnknownApiError unknownApiError = new(testMessage, 0);
Assert.Equal(testMessage, unknownApiError.Message);

// Test the base general error constructors
// Does not exist

Expand Down Expand Up @@ -176,10 +185,11 @@ public void TestExceptionConstructors()
MissingParameterError missingParameterError = new(testPropertyName);
Assert.Equal(string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.MissingRequiredParameter, testPropertyName), missingParameterError.Message);

MissingPropertyError missingPropertyError = new(testObj, testPropertyName);
#pragma warning disable CA2241
Assert.Equal(string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.MissingProperty, new object[] { testObj.GetType().Name, testPropertyName }), missingPropertyError.Message);
#pragma warning restore CA2241
object testObject = new List<string>();
MissingPropertyError missingPropertyError = new(testObject, testPropertyName);
#pragma warning disable CA2241 // Provide correct arguments to formatting methods
Assert.Equal(string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.MissingProperty, new object[] { testObject.GetType().Name, testPropertyName }), missingPropertyError.Message);
#pragma warning restore CA2241 // Provide correct arguments to formatting methods

SignatureVerificationError signatureVerificationError = new();
Assert.Equal(Constants.ErrorMessages.InvalidWebhookSignature, signatureVerificationError.Message);
Expand Down Expand Up @@ -301,10 +311,10 @@ public async Task TestKnownApiExceptionGeneration()
Dictionary<int, Type> exceptionsMap = new()
{
{ 0, typeof(ConnectionError) }, // RestSharp returns status code 0 when a connection cannot be established (i.e. no internet access)
{ 100, typeof(UnexpectedHttpError) },
{ 101, typeof(UnexpectedHttpError) },
{ 102, typeof(UnexpectedHttpError) },
{ 103, typeof(UnexpectedHttpError) },
{ 100, typeof(UnknownHttpError) },
{ 101, typeof(UnknownHttpError) },
{ 102, typeof(UnknownHttpError) },
{ 103, typeof(UnknownHttpError) },
{ 300, typeof(RedirectError) },
{ 301, typeof(RedirectError) },
{ 302, typeof(RedirectError) },
Expand Down Expand Up @@ -372,7 +382,7 @@ public async Task TestUnknownApiException1xxGeneration()
// the exception should be of base type ApiError
Assert.Equal(typeof(ApiError), generatedError.GetType().BaseType);
// specifically, the exception should be of type UnexpectedHttpError
Assert.Equal(typeof(UnexpectedHttpError), generatedError.GetType());
Assert.Equal(typeof(UnknownHttpError), generatedError.GetType());
}

[Fact]
Expand All @@ -392,7 +402,7 @@ public async Task TestUnknownApiException3xxGeneration()
// the exception should be of base type ApiError
Assert.Equal(typeof(ApiError), generatedError.GetType().BaseType);
// specifically, the exception should be of type UnexpectedHttpError
Assert.Equal(typeof(UnexpectedHttpError), generatedError.GetType());
Assert.Equal(typeof(UnknownHttpError), generatedError.GetType());
}

[Fact]
Expand All @@ -412,7 +422,7 @@ public async Task TestUnknownApiException4xxGeneration()
// the exception should be of base type ApiError
Assert.Equal(typeof(ApiError), generatedError.GetType().BaseType);
// specifically, the exception should be of type UnknownApiError
Assert.Equal(typeof(UnknownApiError), generatedError.GetType());
Assert.Equal(typeof(UnknownHttpError), generatedError.GetType());
}

[Fact]
Expand All @@ -432,7 +442,7 @@ public async Task TestUnknownApiException5xxGeneration()
// the exception should be of base type ApiError
Assert.Equal(typeof(ApiError), generatedError.GetType().BaseType);
// specifically, the exception should be of type UnexpectedHttpError
Assert.Equal(typeof(UnexpectedHttpError), generatedError.GetType());
Assert.Equal(typeof(UnknownHttpError), generatedError.GetType());
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using EasyPost.Exceptions;
using EasyPost.Exceptions.API;
using EasyPost.Exceptions.General;
using EasyPost.Http;
Expand Down
18 changes: 10 additions & 8 deletions EasyPost/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ public static class Constants
{
private static readonly Dictionary<int, Type?> HttpExceptionsMap = new()
{
// 1xx status codes are usually HTTP process-related, not server-related
// (mostly) everything else is related to the server being interacted with: https://httpwg.org/specs/rfc9110.html#rfc.section.15.5
{ 0, typeof(ConnectionError) }, // RestSharp returns status code 0 when a connection cannot be established (i.e. no internet access)
{ 100, typeof(UnexpectedHttpError) },
{ 101, typeof(UnexpectedHttpError) },
{ 102, typeof(UnexpectedHttpError) },
{ 103, typeof(UnexpectedHttpError) },
{ 100, typeof(UnknownHttpError) },
{ 101, typeof(UnknownHttpError) },
{ 102, typeof(UnknownHttpError) },
{ 103, typeof(UnknownHttpError) },
{ 300, typeof(RedirectError) },
{ 301, typeof(RedirectError) },
{ 302, typeof(RedirectError) },
Expand Down Expand Up @@ -51,10 +53,10 @@ public static class Constants
Type? exceptionType = null;
SwitchCase @switch = new()
{
{ Utilities.Internal.Extensions.Http.StatusCodeIs1xx(statusCode), () => { exceptionType = typeof(UnexpectedHttpError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs3xx(statusCode), () => { exceptionType = typeof(UnexpectedHttpError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs4xx(statusCode), () => { exceptionType = typeof(UnknownApiError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs5xx(statusCode), () => { exceptionType = typeof(UnexpectedHttpError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs1xx(statusCode), () => { exceptionType = typeof(UnknownHttpError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs3xx(statusCode), () => { exceptionType = typeof(UnknownHttpError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs4xx(statusCode), () => { exceptionType = typeof(UnknownHttpError); } },
{ Utilities.Internal.Extensions.Http.StatusCodeIs5xx(statusCode), () => { exceptionType = typeof(UnknownHttpError); } },
{ SwitchCaseScenario.Default, () => { exceptionType = null; } },
};
@switch.MatchFirst(true); // evaluate switch case, checking which expression evaluates to "true"
Expand Down
6 changes: 0 additions & 6 deletions EasyPost/EasyPost.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,5 @@
<PackagePath>/</PackagePath>
</None>
</ItemGroup>
<ItemGroup>
<Compile Remove="BetaFeatures\Parameters\CarrierAccounts\All.cs" />
</ItemGroup>
<ItemGroup>
<Folder Include="BetaFeatures\Parameters\ApiKeys" />
</ItemGroup>

</Project>
20 changes: 0 additions & 20 deletions EasyPost/Exceptions/API/ExternalApiError.cs

This file was deleted.

20 changes: 0 additions & 20 deletions EasyPost/Exceptions/API/UnexpectedHttpError.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@

namespace EasyPost.Exceptions.API
{
public class UnknownApiError : ApiError
public class UnknownHttpError : ApiError
{
/// <summary>
/// Initializes a new instance of the <see cref="UnknownApiError" /> class.
/// Initializes a new instance of the <see cref="UnknownHttpError" /> class.
/// </summary>
/// <param name="errorMessage">Error message string to print to console.</param>
/// <param name="statusCode">Optional HTTP status code to store as a property.</param>
/// <param name="errorType">Optional error type string to store as a property.</param>
/// <param name="errors">Optional list of Error objects to store as a property.</param>
internal UnknownApiError(string errorMessage, int statusCode, string? errorType = null, List<Error>? errors = null)
internal UnknownHttpError(string errorMessage, int statusCode, string? errorType = null, List<Error>? errors = null)
: base(errorMessage, statusCode, errorType, errors)
{
}
Expand Down
4 changes: 2 additions & 2 deletions EasyPost/Exceptions/API/_Base.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class ApiError : EasyPostError
/// Gets a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public string PrettyPrint
public override string PrettyPrint
{
get
{
Expand Down Expand Up @@ -108,7 +108,7 @@ internal static async Task<ApiError> FromErrorResponse(HttpResponseMessage respo
if (exceptionType == null)
{
// A unaccounted-for status code was in the response.
throw new EasyPostError(string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.UnexpectedHttpStatusCode, statusCodeInt));
throw new UnknownHttpError(string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.UnexpectedHttpStatusCode, statusCodeInt), statusCodeInt);
}
#pragma warning restore IDE0270 // Simplify null check

Expand Down
27 changes: 27 additions & 0 deletions EasyPost/Exceptions/ExternalApiError.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
namespace EasyPost.Exceptions
{
public class ExternalApiError : EasyPostError
{
public readonly string? Code;
public readonly int? StatusCode;

/// <summary>
/// Initializes a new instance of the <see cref="ExternalApiError" /> class.
/// </summary>
/// <param name="errorMessage">Error message string to print to console.</param>
/// <param name="statusCode">Optional HTTP status code to store as a property.</param>
/// <param name="errorType">Optional error type string to store as a property.</param>
internal ExternalApiError(string errorMessage, int statusCode, string? errorType = null)
: base(errorMessage)
{
StatusCode = statusCode;
Code = errorType;
}

/// <summary>
/// Get a formatted error string with expanded details about the API error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => $@"{Code} ({StatusCode}): {Message}";
}
}
6 changes: 6 additions & 0 deletions EasyPost/Exceptions/General/EndOfPaginationError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,11 @@ internal EndOfPaginationError()
: base(Constants.ErrorMessages.NoMorePagesToRetrieve)
{
}

/// <summary>
/// Get a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => Message;
}
}
6 changes: 6 additions & 0 deletions EasyPost/Exceptions/General/FilteringError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,11 @@ internal FilteringError(string message)
: base(message)
{
}

/// <summary>
/// Get a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => Message;
}
}
6 changes: 6 additions & 0 deletions EasyPost/Exceptions/General/InvalidObjectError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,11 @@ internal InvalidObjectError(string message)
: base(message)
{
}

/// <summary>
/// Get a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => Message;
}
}
6 changes: 6 additions & 0 deletions EasyPost/Exceptions/General/JsonError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ internal JsonError(string message)
: base(message)
{
}

/// <summary>
/// Get a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => Message;
}

public class JsonDeserializationError : JsonError
Expand Down
6 changes: 6 additions & 0 deletions EasyPost/Exceptions/General/MissingPropertyError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,11 @@ internal MissingPropertyError(object obj, object propertyName)
#pragma warning restore CA2241
{
}

/// <summary>
/// Get a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => Message;
}
}
6 changes: 6 additions & 0 deletions EasyPost/Exceptions/General/SignatureVerificationError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,11 @@ internal SignatureVerificationError()
: base(Constants.ErrorMessages.InvalidWebhookSignature)
{
}

/// <summary>
/// Get a formatted error string with expanded details about the error.
/// </summary>
/// <returns>A formatted error string.</returns>
public override string PrettyPrint => Message;
}
}
Loading

0 comments on commit 022ebcb

Please sign in to comment.