Improvements to error handling (#323)

- New exception classes with multi-level inheritance - Don't initialize HTTP/API error instances directly, use ".FromResponse" method - Helper methods to centralize the logic to determine which type of HTTP/API error to throw - Utilities available to devs and end-users to see which exception will be triggered by which status code - Unit tests to test HTTP exception generation (for both planned and unplanned status codes) - Fix some unit tests (what exceptions to expect) - Improvements to custom switch case and enum mechanics (all enums in this library will use the custom enum class for consistency) - Make static strings for error messages
EasyPost · Sep 16, 2022 · b90d50b · b90d50b
1 parent b94292b
commit b90d50b
Show file tree

Hide file tree

Showing 49 changed files with 1,037 additions and 192 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,11 @@
   - Improved accessibility levels of internal functions, to prevent accidental use by end users.
   - Files have been organized into a more logical structure.
   - Methods and properties have been organized (e.g. methods ordered by CRUD, properties ordered alphabetically).
+- Consistent exception handling
+  - All exceptions inherit from `EasyPostError`.
+  - API- and HTTP-related exceptions will throw an `ApiError` or inherited-type exception.
+  - API exception types can be retrieved by HTTP status code via the `EasyPost.Exceptions.Constants` class (i.e. to anticipate what error will be thrown for a 404, etc.)
+  - Common exception messages and templates can be found in the `EasyPost.Exceptions.Constants` class (i.e. for log parsing).
 - Dependencies updated to latest versions, including `RestSharp` v108.
 
 ## v3.4.0 (2022-08-02)

diff --git a/EasyPost.Tests/ErrorTest.cs b/EasyPost.Tests/ErrorTest.cs
@@ -1,5 +1,12 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
+using System.Net;
 using System.Threading.Tasks;
+using EasyPost.Exceptions;
+using EasyPost.Exceptions.API;
+using EasyPost.Exceptions.General;
+using EasyPost.Models.API;
+using RestSharp;
 using Xunit;
 
 namespace EasyPost.Tests
@@ -19,7 +26,7 @@ public async Task TestBadParameters()
             {
                 var _ = await Client.Shipment.Create(new Dictionary<string, object>());
             }
-            catch (HttpException error)
+            catch (InvalidRequestError error)
             {
                 Assert.Equal(422, error.StatusCode);
                 Assert.Equal("PARAMETER.REQUIRED", error.Code);
@@ -29,10 +36,127 @@ public async Task TestBadParameters()
         }
 
         [Fact(Skip = "Test is no longer valid")]
-        public Task TestEmptyApiKey()
+        public void TestEmptyApiKey()
         {
             // No longer possible to have an empty API key
+        }
+
+        [Fact]
+        public Task TestKnownApiExceptionGeneration()
+        {
+            // all the error status codes the library should be able to handle
+            Dictionary<int, Type> exceptionsMap = new Dictionary<int, Type>
+            {
+                { 0, typeof(VcrError) }, // EasyVCR uses 0 as the status code when a recording cannot be found
+                { 401, typeof(UnauthorizedError) },
+                { 402, typeof(PaymentError) },
+                { 403, typeof(UnauthorizedError) },
+                { 404, typeof(NotFoundError) },
+                { 405, typeof(MethodNotAllowedError) },
+                { 408, typeof(TimeoutError) },
+                { 422, typeof(InvalidRequestError) },
+                { 429, typeof(RateLimitError) },
+                { 500, typeof(InternalServerError) },
+                { 503, typeof(ServiceUnavailableError) },
+                { 504, typeof(GatewayTimeoutError) }
+            };
+            foreach (KeyValuePair<int, Type> exceptionDetails in exceptionsMap)
+            {
+                int statusCodeInt = exceptionDetails.Key;
+                Type exceptionType = exceptionDetails.Value;
+
+                // Generate a dummy RestResponse with the given status code to parse
+                HttpStatusCode statusCode = (HttpStatusCode)Enum.Parse(typeof(HttpStatusCode), statusCodeInt.ToString());
+                RestResponse response = new RestResponse { StatusCode = statusCode };
+
+                ApiError generatedError = ApiError.FromErrorResponse(response);
+
+                // should be of base type ApiError
+                Assert.Equal(typeof(ApiError), generatedError.GetType().BaseType);
+                // should be specifically of the type we expect
+                Assert.Equal(exceptionType, generatedError.GetType());
+                // should have the correct status code
+                Assert.Equal(statusCodeInt, generatedError.StatusCode);
+                // should have a message
+                Assert.NotNull(generatedError.Message);
+                // because we're not giving it a real API response to parse, there's no errors to extract, so the errors list should be null
+                // this inherently tests that the property exists as well
+                Assert.Null(generatedError.Errors);
+                // should have a code
+                Assert.NotNull(generatedError.Code);
+                // because we're not giving it a real API response to parse, there's no code to extract, so the code should be the default
+                Assert.Equal("RESPONSE.PARSE_ERROR", generatedError.Code);
+            }
+
             return Task.CompletedTask;
         }
+
+        [Fact]
+        public void TestUnknownApiException4xxGeneration()
+        {
+            // library does not have a specific exception for this status code
+            // Since it's a 4xx error, it should throw an UnknownApiError
+            const int unexpectedStatusCode = 418;
+
+            // Generate a dummy RestResponse with the given status code to parse
+            HttpStatusCode statusCode = (HttpStatusCode)Enum.Parse(typeof(HttpStatusCode), unexpectedStatusCode.ToString());
+            RestResponse response = new RestResponse { StatusCode = statusCode };
+
+            ApiError generatedError = ApiError.FromErrorResponse(response);
+
+            // 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());
+        }
+
+        [Fact]
+        public void TestUnknownApiException5xxGeneration()
+        {
+            // library does not have a specific exception for this status code
+            // Since it's a 5xx error, it should throw an UnexpectedHttpError
+            const int unexpectedStatusCode = 502;
+
+            // Generate a dummy RestResponse with the given status code to parse
+            HttpStatusCode statusCode = (HttpStatusCode)Enum.Parse(typeof(HttpStatusCode), unexpectedStatusCode.ToString());
+            RestResponse response = new RestResponse { StatusCode = statusCode };
+
+            ApiError generatedError = ApiError.FromErrorResponse(response);
+
+            // 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());
+        }
+
+        [Fact]
+        public void TestExceptionMessageFormatting()
+        {
+            Type type = typeof(Address);
+            JsonError jsonError = new JsonDeserializationError(type);
+
+            string expectedMessage = string.Format(Constants.ErrorMessages.JsonDeserializationError, type.FullName);
+
+            Assert.Equal(expectedMessage, jsonError.Message);
+        }
+
+        [Fact]
+        public void TestApiExceptionPrettyPrint()
+        {
+            const int statusCode = 401;
+
+            // Generate a dummy RestResponse with the given status code to parse
+            HttpStatusCode httpStatusCode = (HttpStatusCode)Enum.Parse(typeof(HttpStatusCode), statusCode.ToString());
+            RestResponse response = new RestResponse { StatusCode = httpStatusCode };
+
+            ApiError generatedError = ApiError.FromErrorResponse(response);
+
+            // unfortunately, we can't easily load error-related JSON data into the response for parsing, so the pretty print is going to fallback to default values.
+            string expectedMessage = $@"{Constants.ErrorMessages.ApiErrorDetailsParsingError} ({statusCode}): {Constants.ErrorMessages.ApiDidNotReturnErrorDetails}";
+
+            string prettyPrintedError = generatedError.PrettyPrint;
+
+            Assert.Equal(expectedMessage, prettyPrintedError);
+        }
     }
 }
diff --git a/EasyPost.Tests/OrderTest.cs b/EasyPost.Tests/OrderTest.cs
@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using EasyPost.Models.API;
 using EasyPost.Utilities.Annotations;
@@ -70,7 +69,7 @@ public async Task TestLowestRate()
 
             // test lowest rate with carrier filter (should error due to bad carrier)
             List<string> carriers = new List<string> { "BAD_CARRIER" };
-            Assert.Throws<Exception>(() => order.LowestRate(carriers));
+            Assert.Throws<Exceptions.General.FilteringError>(() => order.LowestRate(carriers));
         }
 
         [Fact]

diff --git a/EasyPost.Tests/PickupTest.cs b/EasyPost.Tests/PickupTest.cs
@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using EasyPost.Models.API;
 using EasyPost.Utilities.Annotations;
@@ -44,11 +43,11 @@ public async Task TestLowestRate()
 
             // test lowest rate with service filter (should error due to bad service)
             List<string> services = new List<string> { "BAD_SERVICE" };
-            Assert.Throws<Exception>(() => pickup.LowestRate(null, services));
+            Assert.Throws<Exceptions.General.FilteringError>(() => pickup.LowestRate(null, services));
 
             // test lowest rate with carrier filter (should error due to bad carrier)
             List<string> carriers = new List<string> { "BAD_CARRIER" };
-            Assert.Throws<Exception>(() => pickup.LowestRate(carriers));
+            Assert.Throws<Exceptions.General.FilteringError>(() => pickup.LowestRate(carriers));
         }
 
         [Fact]

diff --git a/EasyPost.Tests/ShipmentTest.cs b/EasyPost.Tests/ShipmentTest.cs
@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
 using EasyPost.Models.API;
@@ -181,7 +180,7 @@ public async Task TestInstanceLowestSmartrate()
             Assert.Equal("USPS", lowestSmartrate.Carrier);
 
             // test lowest smartrate with invalid filters (should error due to strict delivery_days)
-            await Assert.ThrowsAsync<Exception>(async () => await shipment.LowestSmartrate(0, SmartrateAccuracy.Percentile90));
+            await Assert.ThrowsAsync<Exceptions.General.FilteringError>(async () => await shipment.LowestSmartrate(0, SmartrateAccuracy.Percentile90));
 
             // test lowest smartrate with invalid filters (should error due to bad delivery_accuracy)
             // this test is not needed in the C# CL because it uses enums for the accuracy (can't pass in an incorrect value)
@@ -210,7 +209,7 @@ public async Task TestLowestRate()
 
             // test lowest rate with carrier filter (should error due to bad carrier)
             List<string> carriers = new List<string> { "BAD_CARRIER" };
-            Assert.Throws<Exception>(() => shipment.LowestRate(carriers));
+            Assert.Throws<Exceptions.General.FilteringError>(() => shipment.LowestRate(carriers));
         }
 
         [Fact]
@@ -266,7 +265,7 @@ public async Task TestStaticLowestSmartrate()
             Assert.Equal("USPS", lowestSmartrate.Carrier);
 
             // test lowest smartrate with invalid filters (should error due to strict delivery_days)
-            Assert.Throws<Exception>(() => ShipmentService.GetLowestSmartrate(smartrates, 0, SmartrateAccuracy.Percentile90));
+            Assert.Throws<Exceptions.General.FilteringError>(() => ShipmentService.GetLowestSmartrate(smartrates, 0, SmartrateAccuracy.Percentile90));
 
             // test lowest smartrate with invalid filters (should error due to bad delivery_accuracy)
             // this test is not needed in the C# CL because it uses enums for the accuracy (can't pass in an incorrect value)

diff --git a/EasyPost.Tests/TestUtils.cs b/EasyPost.Tests/TestUtils.cs
@@ -3,6 +3,7 @@
 using System.IO;
 using System.Net.Http;
 using System.Runtime.CompilerServices;
+using EasyPost.Exceptions;
 using EasyVCR;
 
 namespace EasyPost.Tests
@@ -57,7 +58,7 @@ internal static string GetApiKey(ApiKey apiKey)
                     keyName = "EASYPOST_PROD_API_KEY";
                     break;
                 default:
-                    throw new Exception("Invalid ApiKey type");
+                    throw new Exception(Constants.ErrorMessages.InvalidApiKeyType);
             }
 
             return Environment.GetEnvironmentVariable(keyName) ?? ApiKeyFailedToPull; // if can't pull from environment, will use a fake key. Won't matter on replay.

diff --git a/EasyPost/Calculation/Rates.cs b/EasyPost/Calculation/Rates.cs
@@ -1,6 +1,7 @@
-using System;
 using System.Collections.Generic;
 using System.Linq;
+using EasyPost.Exceptions;
+using EasyPost.Exceptions.General;
 using EasyPost.Models.API;
 
 namespace EasyPost.Calculation
@@ -41,7 +42,7 @@ public static Rate GetLowestObjectRate(IEnumerable<Rate> rates, List<string>? in
 
                 if (rate.Price == null || lowestRate.Price == null)
                 {
-                    throw new Exception("Could not compare null elements.");
+                    throw new FilteringError(Constants.ErrorMessages.NullObjectComparison);
                 }
 
                 float rateValue = float.Parse(rate.Price);
@@ -58,7 +59,7 @@ public static Rate GetLowestObjectRate(IEnumerable<Rate> rates, List<string>? in
 
             if (lowestRate == null)
             {
-                throw new Exception("No rates found.");
+                throw new FilteringError(string.Format(Constants.ErrorMessages.NoObjectFound, "rates"));
             }
 
             return lowestRate;
@@ -95,7 +96,7 @@ public static Smartrate GetLowestShipmentSmartrate(IEnumerable<Smartrate> smartr
 
             if (lowestSmartrate == null)
             {
-                throw new Exception("No smartrates found.");
+                throw new FilteringError(string.Format(Constants.ErrorMessages.NoObjectFound, "smartrates"));
             }
 
             return lowestSmartrate;

diff --git a/EasyPost/Exception.cs b/EasyPost/Exception.cs