Merge pull request #56 from exceptionless/remove-exception-throwing-from-clients

pc9795 · web-flow · commit b63731c0b67f · 2021-05-20T22:54:28.000+01:00
Remove exception throwing from clients
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/ExceptionlessClient.java b/src/main/java/com/exceptionless/exceptionlessclient/ExceptionlessClient.java
@@ -2,14 +2,14 @@
 
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
 import com.exceptionless.exceptionlessclient.configuration.ConfigurationManager;
+import com.exceptionless.exceptionlessclient.enums.EventPropertyKey;
+import com.exceptionless.exceptionlessclient.enums.EventType;
 import com.exceptionless.exceptionlessclient.models.Event;
 import com.exceptionless.exceptionlessclient.models.EventPluginContext;
 import com.exceptionless.exceptionlessclient.models.PluginContext;
 import com.exceptionless.exceptionlessclient.models.UserDescription;
-import com.exceptionless.exceptionlessclient.enums.EventPropertyKey;
-import com.exceptionless.exceptionlessclient.enums.EventType;
-import com.exceptionless.exceptionlessclient.submission.SubmissionResponse;
 import com.exceptionless.exceptionlessclient.plugins.EventPluginRunner;
+import com.exceptionless.exceptionlessclient.submission.SubmissionResponse;
 import com.exceptionless.exceptionlessclient.utils.VisibleForTesting;
 import lombok.Builder;
 import lombok.Getter;
@@ -269,9 +269,16 @@ public SubmissionResponse updateEmailAndDescription(
             .postUserDescription(
                 referenceId,
                 UserDescription.builder().description(description).emailAddress(email).build());
+    if (response.hasException()) {
+      log.error(
+          String.format("Failed to submit user email and description for event: %s", referenceId),
+          response.getException());
+    }
     if (!response.isSuccess()) {
       log.error(
-          String.format("Failed to submit user email and description for event: %s", referenceId));
+          String.format(
+              "Failed to submit user email and description for event: %s, code: %s",
+              referenceId, response.getCode()));
     }
 
     return response;
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/exceptions/SettingsClientException.java b/src/main/java/com/exceptionless/exceptionlessclient/exceptions/SettingsClientException.java
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/exceptions/SubmissionClientException.java b/src/main/java/com/exceptionless/exceptionlessclient/exceptions/SubmissionClientException.java
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/queue/DefaultEventQueue.java b/src/main/java/com/exceptionless/exceptionlessclient/queue/DefaultEventQueue.java
@@ -1,12 +1,11 @@
 package com.exceptionless.exceptionlessclient.queue;
 
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
-import com.exceptionless.exceptionlessclient.exceptions.SubmissionClientException;
 import com.exceptionless.exceptionlessclient.models.Event;
 import com.exceptionless.exceptionlessclient.models.storage.StorageItem;
-import com.exceptionless.exceptionlessclient.submission.SubmissionResponse;
 import com.exceptionless.exceptionlessclient.storage.StorageProviderIF;
 import com.exceptionless.exceptionlessclient.submission.SubmissionClientIF;
+import com.exceptionless.exceptionlessclient.submission.SubmissionResponse;
 import com.exceptionless.exceptionlessclient.utils.VisibleForTesting;
 import lombok.Builder;
 import lombok.extern.slf4j.Slf4j;
@@ -137,11 +136,13 @@ public void process() {
       log.info(
           String.format("Sending %s events to %s", events.size(), configuration.getServerUrl()));
       SubmissionResponse response = submissionClient.postEvents(events);
+      if (response.hasException()) {
+        log.error("Error submitting events from queue", response.getException());
+        suspendProcessing();
+        return;
+      }
       processSubmissionResponse(response, storedEvents);
       eventPosted(response, events);
-    } catch (SubmissionClientException e) {
-      log.error("Error submitting events from queue", e);
-      suspendProcessing();
     } finally {
       synchronized (this) {
         processingQueue = false;
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/settings/DefaultSettingsClient.java b/src/main/java/com/exceptionless/exceptionlessclient/settings/DefaultSettingsClient.java
@@ -1,7 +1,6 @@
 package com.exceptionless.exceptionlessclient.settings;
 
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
-import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
 import com.exceptionless.exceptionlessclient.utils.Utils;
 import com.exceptionless.exceptionlessclient.utils.VisibleForTesting;
 import com.fasterxml.jackson.core.type.TypeReference;
@@ -64,7 +63,7 @@ public SettingsResponse getSettings(long version) {
           .settings(serverSettings)
           .build();
     } catch (Exception e) {
-      throw new SettingsClientException(e);
+      return SettingsResponse.builder().exception(e).build();
     }
   }
 }
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/settings/SettingsManager.java b/src/main/java/com/exceptionless/exceptionlessclient/settings/SettingsManager.java
@@ -1,6 +1,5 @@
 package com.exceptionless.exceptionlessclient.settings;
 
-import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
 import com.exceptionless.exceptionlessclient.models.storage.StorageItem;
 import com.exceptionless.exceptionlessclient.storage.StorageProviderIF;
 import lombok.Builder;
@@ -80,8 +79,6 @@ public void updateSettings() {
       storageProvider.getSettings().save(response.getSettings());
       propertyChangeSupport.firePropertyChange(
           Property.SETTINGS, prevValue, response.getSettings());
-    } catch (SettingsClientException e) {
-      log.error(String.format("Error retrieving settings for v%s", getVersion()), e);
     } finally {
       synchronized (this) {
         updatingSettings = false;
@@ -90,6 +87,12 @@ public void updateSettings() {
   }
 
   private boolean shouldNotUpdate(SettingsResponse response) {
+    if (response.hasException()) {
+      log.error(
+          String.format("Error retrieving settings for v%s", getVersion()),
+          response.getException());
+      return true;
+    }
     if (response.isNotModified()) {
       log.debug("No need to update, settings are not modified");
       return true;
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/settings/SettingsResponse.java b/src/main/java/com/exceptionless/exceptionlessclient/settings/SettingsResponse.java
@@ -9,6 +9,7 @@ public class SettingsResponse {
   int code;
   String body;
   ServerSettings settings;
+  Exception exception;
 
   public boolean isSuccess() {
     return code >= 200 && code <= 299;
@@ -17,4 +18,8 @@ public boolean isSuccess() {
   public boolean isNotModified() {
     return code == 304;
   }
+
+  public boolean hasException() {
+    return exception != null;
+  }
 }
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/submission/DefaultSubmissionClient.java b/src/main/java/com/exceptionless/exceptionlessclient/submission/DefaultSubmissionClient.java
@@ -1,7 +1,6 @@
 package com.exceptionless.exceptionlessclient.submission;
 
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
-import com.exceptionless.exceptionlessclient.exceptions.SubmissionClientException;
 import com.exceptionless.exceptionlessclient.models.Event;
 import com.exceptionless.exceptionlessclient.models.UserDescription;
 import com.exceptionless.exceptionlessclient.settings.SettingsManager;
@@ -84,7 +83,7 @@ private SubmissionResponse postSubmission(String url, Object data) {
           .body(body == null ? "" : body.string())
           .build();
     } catch (Exception e) {
-      throw new SubmissionClientException(e);
+      return SubmissionResponse.builder().exception(e).build();
     }
   }
 
@@ -122,7 +121,7 @@ public void sendHeartBeat(String sessionIdOrUserId, boolean closeSession) {
                 sessionIdOrUserId));
       }
     } catch (Exception e) {
-      throw new SubmissionClientException(e);
+      log.error("Error while submitting heartbeat", e);
     }
   }
 }
diff --git a/src/main/java/com/exceptionless/exceptionlessclient/submission/SubmissionResponse.java b/src/main/java/com/exceptionless/exceptionlessclient/submission/SubmissionResponse.java
@@ -8,6 +8,7 @@
 public class SubmissionResponse {
   int code;
   String body;
+  Exception exception;
 
   public boolean isSuccess() {
     return code >= 200 && code <= 299;
@@ -36,4 +37,8 @@ public boolean isNotFound() {
   public boolean isRequestEntityTooLarge() {
     return code == 413;
   }
+
+  public boolean hasException() {
+    return exception != null;
+  }
 }
diff --git a/src/test/java/com/exceptionless/exceptionlessclient/queue/DefaultEventQueueTest.java b/src/test/java/com/exceptionless/exceptionlessclient/queue/DefaultEventQueueTest.java
@@ -2,12 +2,11 @@
 
 import com.exceptionless.exceptionlessclient.TestFixtures;
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
-import com.exceptionless.exceptionlessclient.exceptions.SubmissionClientException;
 import com.exceptionless.exceptionlessclient.models.Event;
-import com.exceptionless.exceptionlessclient.submission.SubmissionResponse;
 import com.exceptionless.exceptionlessclient.storage.InMemoryStorage;
 import com.exceptionless.exceptionlessclient.storage.InMemoryStorageProvider;
 import com.exceptionless.exceptionlessclient.submission.DefaultSubmissionClient;
+import com.exceptionless.exceptionlessclient.submission.SubmissionResponse;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -111,7 +110,9 @@ public void itShouldNotPostEmptyEvents() {
   public void itShouldSuspendProcessingOnClientException() {
     storage.save(event);
 
-    doThrow(new SubmissionClientException("test")).when(submissionClient).postEvents(List.of(event));
+    doReturn(SubmissionResponse.builder().exception(new RuntimeException("test")).build())
+        .when(submissionClient)
+        .postEvents(List.of(event));
 
     queue.onEventsPosted(testHandler);
     queue.process();
@@ -250,11 +251,9 @@ public void itShouldReduceSubmissionBatchSizeIfRequestEntitiesAreTooLarge() {
     queue.process();
 
     // One invocation with full batch
-    verify(submissionClient, times(1))
-        .postEvents(argThat(argument -> argument.size() == 3));
+    verify(submissionClient, times(1)).postEvents(argThat(argument -> argument.size() == 3));
     // One invocation with reduced batch
-    verify(submissionClient, times(1))
-        .postEvents(argThat(argument -> argument.size() == 2));
+    verify(submissionClient, times(1)).postEvents(argThat(argument -> argument.size() == 2));
   }
 
   @Test
@@ -303,11 +302,9 @@ public void itShouldResetSubmissionBatchSizeOnNextSuccessfulResponse() {
 
     // Two invocations with full batch; First with the default size and next after a successful
     // response
-    verify(submissionClient, times(2))
-        .postEvents(argThat(argument -> argument.size() == 3));
+    verify(submissionClient, times(2)).postEvents(argThat(argument -> argument.size() == 3));
     // One invocation with reduced batch
-    verify(submissionClient, times(1))
-        .postEvents(argThat(argument -> argument.size() == 2));
+    verify(submissionClient, times(1)).postEvents(argThat(argument -> argument.size() == 2));
   }
 
   @Test
diff --git a/src/test/java/com/exceptionless/exceptionlessclient/settings/DefaultSettingsClientTest.java b/src/test/java/com/exceptionless/exceptionlessclient/settings/DefaultSettingsClientTest.java
@@ -2,7 +2,6 @@
 
 import com.exceptionless.exceptionlessclient.TestFixtures;
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
-import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
 import okhttp3.*;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -14,7 +13,6 @@
 import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.doReturn;
@@ -101,12 +99,13 @@ public void itCanHandleNullSettingsReturnedByTheServer() throws IOException {
   }
 
   @Test
-  public void itCanMapAnyExceptionToSettingsClientException() {
-    RuntimeException e = new RuntimeException("test");
+  public void itCanHandleAllExceptions() {
+    Exception e = new RuntimeException("test");
     doThrow(e).when(httpClient).newCall(any());
 
-    assertThatThrownBy(() -> settingsClient.getSettings(1))
-        .isInstanceOf(SettingsClientException.class)
-        .hasMessage("java.lang.RuntimeException: test");
+    SettingsResponse response = settingsClient.getSettings(1);
+
+    assertThat(response.hasException()).isTrue();
+    assertThat(response.getException()).isSameAs(e);
   }
 }
diff --git a/src/test/java/com/exceptionless/exceptionlessclient/settings/SettingsManagerTest.java b/src/test/java/com/exceptionless/exceptionlessclient/settings/SettingsManagerTest.java
@@ -1,6 +1,5 @@
 package com.exceptionless.exceptionlessclient.settings;
 
-import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
 import com.exceptionless.exceptionlessclient.storage.InMemoryStorage;
 import com.exceptionless.exceptionlessclient.storage.InMemoryStorageProvider;
 import org.junit.jupiter.api.BeforeEach;
@@ -152,7 +151,9 @@ public void itWillNotSaveSettingsIfNoSettingsReturnedByTheServer() {
 
   @Test
   public void itCanHandleSettingsClientException() {
-    doThrow(new SettingsClientException("test")).when(settingsClient).getSettings(anyLong());
+    doReturn(SettingsResponse.builder().exception(new RuntimeException("test")).build())
+        .when(settingsClient)
+        .getSettings(anyLong());
 
     settingsManager.updateSettings();
   }
diff --git a/src/test/java/com/exceptionless/exceptionlessclient/submission/DefaultSubmissionClientTest.java b/src/test/java/com/exceptionless/exceptionlessclient/submission/DefaultSubmissionClientTest.java
@@ -2,7 +2,6 @@
 
 import com.exceptionless.exceptionlessclient.TestFixtures;
 import com.exceptionless.exceptionlessclient.configuration.Configuration;
-import com.exceptionless.exceptionlessclient.exceptions.SubmissionClientException;
 import com.exceptionless.exceptionlessclient.models.Event;
 import com.exceptionless.exceptionlessclient.models.UserDescription;
 import com.exceptionless.exceptionlessclient.settings.SettingsManager;
@@ -18,7 +17,6 @@
 import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.*;
@@ -90,12 +88,14 @@ public void itCanPostEventsSuccessfullyWhenNoSettingHeaderIsReturned() throws IO
   }
 
   @Test
-  public void itCanThrowAllExceptionsAsSubmissionExceptionWhilePostingEvents() {
-    doThrow(new RuntimeException("test")).when(httpClient).newCall(any());
+  public void itCanHandleAllExceptionsWhilePostingEvents() {
+    Exception exception = new RuntimeException("test");
+    doThrow(exception).when(httpClient).newCall(any());
+
+    SubmissionResponse response = submissionClient.postEvents(List.of(Event.builder().build()));
 
-    assertThatThrownBy(() -> submissionClient.postEvents(List.of(Event.builder().build())))
-        .isInstanceOf(SubmissionClientException.class)
-        .hasMessage("java.lang.RuntimeException: test");
+    assertThat(response.hasException()).isTrue();
+    assertThat(response.getException()).isSameAs(exception);
   }
 
   @Test
@@ -140,15 +140,14 @@ public void itCanPostUserDescriptionSuccessfullyWhenNoSettingHeaderIsReturned()
   }
 
   @Test
-  public void itCanThrowAllExceptionsAsSubmissionExceptionWhilePostingUserDescription() {
-    doThrow(new RuntimeException("test")).when(httpClient).newCall(any());
+  public void itCanHandleAllExceptionsWhilePostingUserDescription() {
+    Exception exception = new RuntimeException("test");
+    doThrow(exception).when(httpClient).newCall(any());
 
-    assertThatThrownBy(
-            () ->
-                submissionClient.postUserDescription(
-                    "test-reference-id", UserDescription.builder().build()))
-        .isInstanceOf(SubmissionClientException.class)
-        .hasMessage("java.lang.RuntimeException: test");
+    SubmissionResponse response = submissionClient.postUserDescription("test-reference-id", UserDescription.builder().build());
+
+    assertThat(response.hasException()).isTrue();
+    assertThat(response.getException()).isSameAs(exception);
   }
 
   @Test
@@ -172,11 +171,9 @@ public void itCanSendHeartbeatSuccessfully() throws IOException {
   }
 
   @Test
-  public void itCanThrowAllExceptionsAsSubmissionExceptionWhileSendingHeartbeat() {
+  public void itCanHandleAllExceptionsWhileSendingHeartbeat() {
     doThrow(new RuntimeException("test")).when(httpClient).newCall(any());
 
-    assertThatThrownBy(() -> submissionClient.sendHeartBeat("test-user-id", true))
-        .isInstanceOf(SubmissionClientException.class)
-        .hasMessage("java.lang.RuntimeException: test");
+    submissionClient.sendHeartBeat("test-user-id", true);
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`package com.exceptionless.exceptionlessclient.settings;`
`2`	`2`
`3`	`3`	`import com.exceptionless.exceptionlessclient.configuration.Configuration;`
`4`		`-import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;`
`5`	`4`	`import com.exceptionless.exceptionlessclient.utils.Utils;`
`6`	`5`	`import com.exceptionless.exceptionlessclient.utils.VisibleForTesting;`
`7`	`6`	`import com.fasterxml.jackson.core.type.TypeReference;`
`@@ -64,7 +63,7 @@ public SettingsResponse getSettings(long version) {`
`64`	`63`	`.settings(serverSettings)`
`65`	`64`	`.build();`
`66`	`65`	`} catch (Exception e) {`
`67`		`- throw new SettingsClientException(e);`
	`66`	`+ return SettingsResponse.builder().exception(e).build();`
`68`	`67`	`}`
`69`	`68`	`}`
`70`	`69`	`}`