fix: raise errors for non-2xx status codes (#352)

- 3xx status code is considered an error - All known 3xx status codes will throw a RedirectError exception - Unknown 3xx status codes will throw a UnexpectedHttpError exception (same as 5xx) - 1xx status codes also count as errors - Allow CI to be run manually (ironically, this will trigger an automatic CI run)
EasyPost · Oct 5, 2022 · 894b284 · 894b284
1 parent 6e46aec
commit 894b284
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 8 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -4,6 +4,7 @@ on:
   push:
     branches: [ master ]
   pull_request: ~
+  workflow_dispatch: ~
 
 jobs:
   lint:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@
 
 - Improved API error parsing
   - API error message may be an array rather than a string. Arrays will be concatenated (by comma) and returned as a string.
+- Capture 1xx and 3xx HTTP status codes as errors
+  - Any known 3xx status code from the EasyPost API will throw a `RedirectError` exception
+  - Any unknown 3xx status code will throw a `UnexpectedHttpError` exception
+  - Any 1xx status code (known or unknown) will throw a `UnexpectedHttpError` exception
 
 ## v4.0.0-rc1 (2022-09-26)
 

diff --git a/EasyPost.Tests/ErrorTest.cs b/EasyPost.Tests/ErrorTest.cs
@@ -48,6 +48,19 @@ public Task TestKnownApiExceptionGeneration()
             Dictionary<int, Type> exceptionsMap = new Dictionary<int, Type>
             {
                 { 0, typeof(VcrError) }, // EasyVCR uses 0 as the status code when a recording cannot be found
+                { 100, typeof(UnexpectedHttpError) },
+                { 101, typeof(UnexpectedHttpError) },
+                { 102, typeof(UnexpectedHttpError) },
+                { 103, typeof(UnexpectedHttpError) },
+                { 300, typeof(RedirectError) },
+                { 301, typeof(RedirectError) },
+                { 302, typeof(RedirectError) },
+                { 303, typeof(RedirectError) },
+                { 304, typeof(RedirectError) },
+                { 305, typeof(RedirectError) },
+                { 306, typeof(RedirectError) },
+                { 307, typeof(RedirectError) },
+                { 308, typeof(RedirectError) },
                 { 401, typeof(UnauthorizedError) },
                 { 402, typeof(PaymentError) },
                 { 403, typeof(UnauthorizedError) },
@@ -91,6 +104,44 @@ public Task TestKnownApiExceptionGeneration()
             return Task.CompletedTask;
         }
 
+        [Fact]
+        public void TestUnknownApiException1xxGeneration()
+        {
+            // library does not have a specific exception for this status code
+            // Since it's a 1xx error, it should throw an UnexpectedHttpError
+            const int unexpectedStatusCode = 199;
+
+            // 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 TestUnknownApiException3xxGeneration()
+        {
+            // library does not have a specific exception for this status code
+            // Since it's a 3xx error, it should throw an UnexpectedHttpError
+            const int unexpectedStatusCode = 319;
+
+            // 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 TestUnknownApiException4xxGeneration()
         {

diff --git a/EasyPost/Exceptions/API/RedirectError.cs b/EasyPost/Exceptions/API/RedirectError.cs
@@ -0,0 +1,19 @@
+using System.Collections.Generic;
+using EasyPost.Models.API;
+
+namespace EasyPost.Exceptions.API
+{
+    public class RedirectError : ApiError
+    {
+        /// <summary>
+        ///     Initializes a new instance of the <see cref="RedirectError" /> 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 RedirectError(string errorMessage, int statusCode, string? errorType = null, List<Error>? errors = null) : base(errorMessage, statusCode, errorType, errors)
+        {
+        }
+    }
+}
diff --git a/EasyPost/Exceptions/API/_Base.cs b/EasyPost/Exceptions/API/_Base.cs
@@ -58,11 +58,11 @@ protected ApiError(string errorMessage, int? statusCode = null, string? errorTyp
         ///     Do not pass a non-error response to this method.
         /// </summary>
         /// <param name="response">RestResponse response to parse</param>
-        /// <raises>EasyPostError if a non-400/500 error code is extracted from the response.</raises>
+        /// <raises>EasyPostError if an unplanned response code is found (i.e. user passed in a non-error RestResponse response object with a 2xx status code).</raises>
         /// <returns>An instance of an HttpError-inherited exception.</returns>
         internal static ApiError FromErrorResponse(RestResponse response)
         {
-            // NOTE: This method anticipates that the status code will be a 4xx or 5xx error code.
+            // NOTE: This method anticipates that the status code will be a non-2xx code.
             // Do not use this method to parse a successful response.
             HttpStatusCode statusCode = response.StatusCode;
             int statusCodeInt = (int)statusCode;
@@ -93,7 +93,7 @@ internal static ApiError FromErrorResponse(RestResponse response)
             }
             catch
             {
-                // could not extract error details from the API response (or API did not return data, i.e. 500) -> HttpError type rather than ApiError
+                // could not extract error details from the API response (or API did not return data, i.e. 1xx, 3xx or 5xx)
                 errorMessage = response.ErrorMessage // fallback to standard HTTP error message
                                ?? response.StatusDescription // fallback to standard HTTP status description
                                ?? Constants.ErrorMessages.ApiDidNotReturnErrorDetails; // fallback to no error details
@@ -104,7 +104,7 @@ internal static ApiError FromErrorResponse(RestResponse response)
             Type? exceptionType = statusCode.EasyPostExceptionType();
             if (exceptionType == null)
             {
-                // A non-400/500 error code in the response, which is not expected.
+                // A unaccounted-for status code was in the response.
                 throw new EasyPostError(string.Format(Constants.ErrorMessages.UnexpectedHttpStatusCode, statusCodeInt));
             }
 

diff --git a/EasyPost/Exceptions/Constants.cs b/EasyPost/Exceptions/Constants.cs
@@ -11,6 +11,19 @@ public static class Constants
         private static readonly Dictionary<int, Type> HttpExceptionsMap = new Dictionary<int, Type>
         {
             { 0, typeof(VcrError) }, // EasyVCR uses 0 as the status code when a recording cannot be found
+            { 100, typeof(UnexpectedHttpError) },
+            { 101, typeof(UnexpectedHttpError) },
+            { 102, typeof(UnexpectedHttpError) },
+            { 103, typeof(UnexpectedHttpError) },
+            { 300, typeof(RedirectError) },
+            { 301, typeof(RedirectError) },
+            { 302, typeof(RedirectError) },
+            { 303, typeof(RedirectError) },
+            { 304, typeof(RedirectError) },
+            { 305, typeof(RedirectError) },
+            { 306, typeof(RedirectError) },
+            { 307, typeof(RedirectError) },
+            { 308, typeof(RedirectError) },
             { 401, typeof(UnauthorizedError) },
             { 402, typeof(PaymentError) },
             { 403, typeof(UnauthorizedError) },
@@ -41,6 +54,8 @@ public static class Constants
             Type? exceptionType = null;
             var @switch = new SwitchCase
             {
+                { Utilities.Http.StatusCodeIs1xx(statusCode), () => { exceptionType = typeof(UnexpectedHttpError); } },
+                { Utilities.Http.StatusCodeIs3xx(statusCode), () => { exceptionType = typeof(UnexpectedHttpError); } },
                 { Utilities.Http.StatusCodeIs4xx(statusCode), () => { exceptionType = typeof(UnknownApiError); } },
                 { Utilities.Http.StatusCodeIs5xx(statusCode), () => { exceptionType = typeof(UnexpectedHttpError); } },
                 { SwitchCaseScenario.Default, () => { exceptionType = null; } }

diff --git a/EasyPost/Utilities/Http.cs b/EasyPost/Utilities/Http.cs
@@ -17,6 +17,36 @@ internal static bool HasStatusCodeBetween(this RestResponseBase response, int mi
             return StatusCodeBetween(response, min, max);
         }
 
+        /// <summary>
+        ///     Return whether the given status code is a 1xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 1xx error, False otherwise.</returns>
+        internal static bool Is1xx(this HttpStatusCode statusCode)
+        {
+            return StatusCodeIs1xx(statusCode);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 2xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 2xx error, False otherwise.</returns>
+        internal static bool Is2xx(this HttpStatusCode statusCode)
+        {
+            return StatusCodeIs2xx(statusCode);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 3xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 3xx error, False otherwise.</returns>
+        internal static bool Is3xx(this HttpStatusCode statusCode)
+        {
+            return StatusCodeIs3xx(statusCode);
+        }
+
         /// <summary>
         ///     Return whether the given status code is a 4xx error.
         /// </summary>
@@ -53,7 +83,7 @@ internal static bool IsBetween(this HttpStatusCode statusCode, int min, int max)
         ///     Return whether the given response has an error status code.
         /// </summary>
         /// <param name="response">Response to check.</param>
-        /// <returns>True if the response code is not in the 200-399 range, false otherwise.</returns>
+        /// <returns>True if the response code is not in the 200-299 range, false otherwise.</returns>
         internal static bool ReturnedError(this RestResponse response)
         {
             return !ReturnedNoError(response);
@@ -63,10 +93,10 @@ internal static bool ReturnedError(this RestResponse response)
         ///     Return whether the given response has a successful status code.
         /// </summary>
         /// <param name="response">Response to check.</param>
-        /// <returns>True if the response code is in the 200-399 range, false otherwise.</returns>
+        /// <returns>True if the response code is in the 200-299 range, false otherwise.</returns>
         internal static bool ReturnedNoError(this RestResponse response)
         {
-            return StatusCodeBetween(response, 100, 399);
+            return response.StatusCode.Is2xx();
         }
 
         /// <summary>
@@ -105,6 +135,66 @@ internal static bool StatusCodeBetween(RestResponseBase response, int min, int m
             return StatusCodeBetween(response.StatusCode, min, max);
         }
 
+        /// <summary>
+        ///     Return whether the given status code is a 1xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 1xx error, False otherwise.</returns>
+        internal static bool StatusCodeIs1xx(int statusCode)
+        {
+            return StatusCodeBetween(statusCode, 100, 199);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 1xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 1xx error, False otherwise.</returns>
+        internal static bool StatusCodeIs1xx(HttpStatusCode statusCode)
+        {
+            return StatusCodeBetween(statusCode, 100, 199);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 2xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 2xx error, False otherwise.</returns>
+        internal static bool StatusCodeIs2xx(int statusCode)
+        {
+            return StatusCodeBetween(statusCode, 200, 299);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 2xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 2xx error, False otherwise.</returns>
+        internal static bool StatusCodeIs2xx(HttpStatusCode statusCode)
+        {
+            return StatusCodeBetween(statusCode, 200, 299);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 3xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 3xx error, False otherwise.</returns>
+        internal static bool StatusCodeIs3xx(int statusCode)
+        {
+            return StatusCodeBetween(statusCode, 300, 399);
+        }
+
+        /// <summary>
+        ///     Return whether the given status code is a 3xx error.
+        /// </summary>
+        /// <param name="statusCode">Status code to check.</param>
+        /// <returns>True if the status code is a 3xx error, False otherwise.</returns>
+        internal static bool StatusCodeIs3xx(HttpStatusCode statusCode)
+        {
+            return StatusCodeBetween(statusCode, 300, 399);
+        }
+
         /// <summary>
         ///     Return whether the given status code is a 4xx error.
         /// </summary>

diff --git a/EasyPost/_base/EasyPostClient.cs b/EasyPost/_base/EasyPostClient.cs
@@ -153,7 +153,7 @@ internal async Task<bool> Request(Method method, string url, ApiVersion apiVersi
             // Execute the request
             RestResponse response = await _restClient.ExecuteAsync(restRequest);
 
-            // Return whether the HTTP request produced an error (4xx or 5xx status code) or not
+            // Return whether the HTTP request produced an error (3xx, 4xx or 5xx status code) or not
             return response.ReturnedNoError();
         }
-Original file line number
+Diff line change
@@ Expand Up / @@ -4,6 +4,7 @@ on: @@
       push:
         branches: [ master ]
       pull_request: ~
+      workflow_dispatch: ~
     jobs:
       lint:
@@ Expand Down @@