Bug fixes (#16)

- Fix bug where HTTP error data is not being stored as expected in cassette, causing empty error stream on replay
- Fix bug with expiration time frames not handling non-never/forever enums correctly (throwing NullPointerException)

Author: nwithan8

CHANGELOG.md

@@ -1,5 +1,15 @@
 # CHANGELOG
 
+## v0.4.2 (2022-10-20)
+
+- Fix a bug where the error data of a bad HTTP request (4xx or 5xx) was not stored as expected in cassettes, causing
+  empty error streams on replay.
+    - Error data for a bad HTTP request is now stored as the "body" in the cassette just like a good HTTP request
+      would, rather than needlessly stored in a separate "error" key. This more closely matches the behavior of EasyVCR C#.
+    - This is a breaking change for previously-recorded "error" cassettes, which will no longer replay as expected and
+      will need to be re-recorded (although likely never worked as expected in the first place).
+- Fix a bug where using any expiration time frame other than "forever" and "never" would throw a NullPointerException.
+
 ## v0.4.1 (2022-10-19)
 
 - Fix a bug where the error stream of a bad HTTP request (4xx or 5xx) was not properly recreated on replay.

VERSION

@@ -1 +1 @@
-0.4.1
+0.4.2

pom.xml

@@ -7,7 +7,7 @@
     <groupId>com.easypost</groupId>
     <artifactId>easyvcr</artifactId>
 
-    <version>0.4.1</version>
+    <version>0.4.2</version>
     <packaging>jar</packaging>
 
     <name>com.easypost:easyvcr</name>

src/main/java/com/easypost/easyvcr/TimeFrame.java

@@ -89,12 +89,19 @@ public boolean hasLapsed(long fromTimeEpochTimestamp) {
      * @return Starting time plus this time frame.
      */
     private Instant timePlusFrame(Instant fromTime) {
+        // We need to do a null check here. The "default" case in the switch statement below doesn't handle null enums.
+        if (commonTimeFrame == null) {  // No common time frame was used
+            return fromTime.plus(days, ChronoUnit.DAYS).plus(hours, ChronoUnit.HOURS).plus(minutes, ChronoUnit.MINUTES)
+                    .plus(seconds, ChronoUnit.SECONDS);
+        }
         switch (commonTimeFrame) {
             case Forever:
                 return Instant.MAX;  // will always been in the future
             case Never:
                 return Instant.MIN; // will always been in the past
             default:
+                // We should never get here, since there should always either be an accounted-for enum,
+                // or a null value (handled above).
                 return fromTime.plus(days, ChronoUnit.DAYS).plus(hours, ChronoUnit.HOURS)
                         .plus(minutes, ChronoUnit.MINUTES).plus(seconds, ChronoUnit.SECONDS);
         }

src/main/java/com/easypost/easyvcr/interactionconverters/HttpUrlConnectionInteractionConverter.java

@@ -80,11 +80,10 @@ public ResponseAndTime createRecordedResponse(HttpURLConnection connection, Cens
             String uriString = connection.getURL().toString();
             Map<String, List<String>> headers = connection.getHeaderFields();
             String body = null;
-            String errors = null;
             try {
                 body = readFromInputStream(connection.getInputStream());
             } catch (NullPointerException | IOException ignored) {  // nothing in body if bad status code from server
-                errors = readFromInputStream(connection.getErrorStream());
+                body = readFromInputStream(connection.getErrorStream());
             }
 
             // apply censors
@@ -101,10 +100,6 @@ public ResponseAndTime createRecordedResponse(HttpURLConnection connection, Cens
                 body = censors.applyBodyParameterCensors(body);
                 response.setBody(body);
             }
-            if (errors != null) {
-                errors = censors.applyBodyParameterCensors(errors);
-                response.setErrors(errors);
-            }
 
             return new ResponseAndTime(response, milliseconds);
         } catch (URISyntaxException | IOException ignored) {

src/main/java/com/easypost/easyvcr/internalutilities/ExpirationActionExtensions.java

@@ -16,7 +16,7 @@ public static void checkCompatibleSettings(ExpirationActions action, Mode mode)
             throws RecordingExpirationException {
         if (action == ExpirationActions.RecordAgain && mode == Mode.Replay) {
             throw new RecordingExpirationException(
-                    "Cannot use the Record_Again expiration action in combination with Replay mode.");
+                    "Cannot use the RecordAgain expiration action in combination with Replay mode.");
         }
     }
 }

src/main/java/com/easypost/easyvcr/requestelements/Response.java

@@ -47,11 +47,6 @@ public final class Response extends HttpElement {
      */
     private Status status;
 
-    /**
-     * The errors of the response.
-     */
-    private String errors;
-
     /**
      * The URI of the response.
      */
@@ -399,24 +394,6 @@ public void setStatus(Status status) {
         this.status = status;
     }
 
-    /**
-     * Returns the errors of the response.
-     *
-     * @return the errors of the response
-     */
-    public String getErrors() {
-        return this.errors;
-    }
-
-    /**
-     * Sets the errors of the response.
-     *
-     * @param errors the errors of the response
-     */
-    public void setErrors(String errors) {
-        this.errors = errors;
-    }
-
     /**
      * Returns the URI of the response.
      *

src/test/java/HttpUrlConnectionTest.java

@@ -27,9 +27,10 @@
 
 import static com.easypost.easyvcr.internalutilities.Tools.readFromInputStream;
 
-public class  HttpUrlConnectionTest {
+public class HttpUrlConnectionTest {
 
-    private static FakeDataService.IPAddressData getIPAddressDataRequest(Cassette cassette, Mode mode) throws Exception {
+    private static FakeDataService.IPAddressData getIPAddressDataRequest(Cassette cassette, Mode mode)
+            throws Exception {
         RecordableHttpsURLConnection connection = TestUtils.getSimpleHttpsURLConnection(cassette.name, mode, null);
 
         FakeDataService.HttpsUrlConnection fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
@@ -42,7 +43,8 @@ public void testPOSTRequest() throws Exception {
         AdvancedSettings advancedSettings = new AdvancedSettings();
         advancedSettings.matchRules = new MatchRules().byMethod().byBody().byFullUrl();
         RecordableHttpsURLConnection connection =
-                TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_post_request", Mode.Record, advancedSettings);
+                TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_post_request", Mode.Record,
+                        advancedSettings);
         String jsonInputString = "{'name': 'Upendra', 'job': 'Programmer'}";
         connection.setDoOutput(true);
         connection.setRequestMethod("POST");
@@ -70,7 +72,8 @@ public void testNonJsonDataWithCensors() throws Exception {
 
         advancedSettings.matchRules = new MatchRules().byMethod().byBody().byFullUrl();
         RecordableHttpsURLConnection connection =
-                TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_non_json", Mode.Record, advancedSettings);
+                TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_non_json", Mode.Record,
+                        advancedSettings);
         String jsonInputString = "{'name': 'Upendra', 'job': 'Programmer'}";
         connection.setDoOutput(true);
         connection.setRequestMethod("POST");
@@ -164,7 +167,8 @@ public void testInteractionElements() throws Exception {
 
         // Most elements of a VCR request are black-boxed, so we can't test them here.
         // Instead, we can get the recreated HttpResponseMessage and check the details.
-        RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
+        RecordableHttpsURLConnection response =
+                (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
         Assert.assertNotNull(response);
     }
 
@@ -193,7 +197,8 @@ public void testCensors() throws Exception {
         connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection,
                 FakeDataService.URL, cassette, Mode.Replay, advancedSettings);
         fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
-        RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
+        RecordableHttpsURLConnection response =
+                (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
 
         // check that the replayed response contains the censored header
         Assert.assertNotNull(response);
@@ -221,7 +226,8 @@ public void testMatchSettings() throws Exception {
         connection.setRequestProperty("X-Custom-Header",
                 "custom-value"); // add custom header to request, shouldn't matter when matching by default rules
         fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
-        RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
+        RecordableHttpsURLConnection response =
+                (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
         Assert.assertNotNull(response);
 
         // replay cassette with custom match rules, should not find a match because request is different (throw exception)
@@ -290,7 +296,8 @@ public void testIgnoreElementsFailMatch() throws URISyntaxException, IOException
         String bodyData2 = "{'name': 'NewName', 'job': 'Programmer'}";
 
         // record baseline request first
-        RecordableHttpsURLConnection connection = HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record);
+        RecordableHttpsURLConnection connection =
+                HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record);
         connection.setDoOutput(true);
         connection.setRequestMethod("POST");
         // use bodyData1 to make request
@@ -339,7 +346,8 @@ public void testIgnoreElementsPassMatch() throws URISyntaxException, IOException
         String bodyData2 = "{'name': 'NewName', 'job': 'Programmer'}";
 
         // record baseline request first
-        RecordableHttpsURLConnection connection = HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record);
+        RecordableHttpsURLConnection connection =
+                HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record);
         connection.setDoOutput(true);
         connection.setRequestMethod("POST");
         // use bodyData1 to make request
@@ -383,7 +391,7 @@ public void testIgnoreElementsPassMatch() throws URISyntaxException, IOException
     }
 
     @Test
-    public void testExpirationSettings() throws Exception {
+    public void testExpirationSettingsCommonTimeFrame() throws Exception {
         Cassette cassette = TestUtils.getCassette("test_expiration_settings");
         cassette.erase(); // Erase cassette before recording
 
@@ -398,7 +406,8 @@ public void testExpirationSettings() throws Exception {
         connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection,
                 FakeDataService.URL, cassette, Mode.Replay);
         fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
-        RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
+        RecordableHttpsURLConnection response =
+                (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
         Assert.assertNotNull(response);
 
         // replay cassette with custom expiration rules, should not find a match because recording is expired (throw exception)
@@ -416,10 +425,49 @@ public void testExpirationSettings() throws Exception {
         // replay cassette with bad expiration rules, should throw an exception because settings are bad
         advancedSettings = new AdvancedSettings();
         advancedSettings.timeFrame = TimeFrame.never();
-        advancedSettings.whenExpired = ExpirationActions.RecordAgain;  // invalid settings for replay mode, should throw exception
+        advancedSettings.whenExpired =
+                ExpirationActions.RecordAgain;  // invalid settings for replay mode, should throw exception
         AdvancedSettings finalAdvancedSettings = advancedSettings;
-        Assert.assertThrows(RecordingExpirationException.class, () -> HttpClients.newClient(HttpClientType.HttpsUrlConnection,
-                FakeDataService.URL, cassette, Mode.Replay, finalAdvancedSettings));
+        Assert.assertThrows(RecordingExpirationException.class,
+                () -> HttpClients.newClient(HttpClientType.HttpsUrlConnection, FakeDataService.URL, cassette,
+                        Mode.Replay, finalAdvancedSettings));
+    }
+
+    @Test
+    public void testExpirationSettingsCustomTimeFrame() throws Exception {
+        Cassette cassette = TestUtils.getCassette("test_expiration_settings");
+        cassette.erase(); // Erase cassette before recording
+
+        // record cassette first
+        RecordableHttpsURLConnection connection =
+                (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection,
+                        FakeDataService.URL, cassette, Mode.Record);
+        FakeDataService.HttpsUrlConnection fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
+        fakeDataService.getIPAddressDataRawResponse();
+
+        // Custom expiration rules
+        AdvancedSettings advancedSettings = new AdvancedSettings();
+        advancedSettings.timeFrame = new TimeFrame(1, 0, 0, 0); // 1 day from now
+        advancedSettings.whenExpired = ExpirationActions.ThrowException;  // throw exception when recording is expired
+
+        // replay cassette with custom expiration rules, should find a match
+        connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection,
+                FakeDataService.URL, cassette, Mode.Replay, advancedSettings);
+        fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
+        RecordableHttpsURLConnection response =
+                (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse();
+        Assert.assertNotNull(response);
+
+        // Change expiration rules
+        advancedSettings.timeFrame = new TimeFrame(-1, 0, 0, 0); // 1 day ago
+
+        // replay cassette with custom expiration rules, should not find a match because recording is expired (throw exception)
+        connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection,
+                FakeDataService.URL, cassette, Mode.Replay, advancedSettings);
+        fakeDataService = new FakeDataService.HttpsUrlConnection(connection);
+        FakeDataService.HttpsUrlConnection finalFakeDataService = fakeDataService;
+        // this throws a RuntimeException rather than a RecordingExpirationException because the exceptions are coalesced internally
+        Assert.assertThrows(Exception.class, () -> finalFakeDataService.getIPAddressData());
     }
 
     @Test