Honor RetryAfter (#236)

sentry-core/src/main/java/io/sentry/core/AsyncConnectionFactory.java

@@ -2,22 +2,18 @@
 
 import io.sentry.core.cache.IEventCache;
 import io.sentry.core.transport.AsyncConnection;
-import io.sentry.core.transport.IBackOffIntervalStrategy;
 
 final class AsyncConnectionFactory {
   private AsyncConnectionFactory() {}
 
   public static AsyncConnection create(SentryOptions options, IEventCache eventCache) {
-    IBackOffIntervalStrategy linearBackoff = attempt -> attempt * 500;
 
     // the connection doesn't do any retries of failed sends and can hold at most the same number
     // of pending events as there are being cached. The rest is dropped.
     return new AsyncConnection(
         options.getTransport(),
         options.getTransportGate(),
-        linearBackoff,
         eventCache,
-        0,
         options.getCacheDirSize(),
         options);
   }

sentry-core/src/main/java/io/sentry/core/transport/AsyncConnection.java

@@ -1,5 +1,7 @@
 package io.sentry.core.transport;
 
+import static io.sentry.core.transport.RetryingThreadPoolExecutor.HTTP_RETRY_AFTER_DEFAULT_DELAY_MS;
+
 import io.sentry.core.SentryEvent;
 import io.sentry.core.SentryLevel;
 import io.sentry.core.SentryOptions;
@@ -28,28 +30,22 @@ public final class AsyncConnection implements Closeable, Connection {
   private final SentryOptions options;
 
   public AsyncConnection(
-      ITransport transport,
-      ITransportGate transportGate,
-      IBackOffIntervalStrategy backOffIntervalStrategy,
-      IEventCache eventCache,
-      int maxRetries,
-      int maxQueueSize,
-      SentryOptions options) {
-    this(
-        transport,
-        transportGate,
-        eventCache,
-        initExecutor(maxRetries, maxQueueSize, backOffIntervalStrategy, eventCache),
-        options);
+      final ITransport transport,
+      final ITransportGate transportGate,
+      final IEventCache eventCache,
+      final int maxQueueSize,
+      final SentryOptions options) {
+    this(transport, transportGate, eventCache, initExecutor(maxQueueSize, eventCache), options);
   }
 
   @TestOnly
   AsyncConnection(
-      ITransport transport,
-      ITransportGate transportGate,
-      IEventCache eventCache,
-      ExecutorService executorService,
-      SentryOptions options) {
+      final ITransport transport,
+      final ITransportGate transportGate,
+      final IEventCache eventCache,
+      final ExecutorService executorService,
+      final SentryOptions options) {
+
     this.transport = transport;
     this.transportGate = transportGate;
     this.eventCache = eventCache;
@@ -58,25 +54,17 @@ public AsyncConnection(
   }
 
   private static RetryingThreadPoolExecutor initExecutor(
-      int maxRetries,
-      int maxQueueSize,
-      IBackOffIntervalStrategy backOffIntervalStrategy,
-      IEventCache eventCache) {
+      final int maxQueueSize, final IEventCache eventCache) {
 
-    RejectedExecutionHandler storeEvents =
+    final RejectedExecutionHandler storeEvents =
         (r, executor) -> {
           if (r instanceof EventSender) {
             eventCache.store(((EventSender) r).event);
           }
         };
 
     return new RetryingThreadPoolExecutor(
-        1,
-        maxRetries,
-        maxQueueSize,
-        new AsyncConnectionThreadFactory(),
-        backOffIntervalStrategy,
-        storeEvents);
+        1, maxQueueSize, new AsyncConnectionThreadFactory(), storeEvents);
   }
 
   /**
@@ -86,9 +74,8 @@ private static RetryingThreadPoolExecutor initExecutor(
    * @throws IOException on error
    */
   @SuppressWarnings("FutureReturnValueIgnored")
-  // https://errorprone.info/bugpattern/FutureReturnValueIgnored
   @Override
-  public void send(SentryEvent event, @Nullable Object hint) throws IOException {
+  public void send(final SentryEvent event, final @Nullable Object hint) throws IOException {
     IEventCache currentEventCache = eventCache;
     if (hint instanceof Cached) {
       currentEventCache = NoOpEventCache.getInstance();
@@ -123,21 +110,23 @@ private static final class AsyncConnectionThreadFactory implements ThreadFactory
     private int cnt;
 
     @Override
-    public Thread newThread(@NotNull Runnable r) {
-      Thread ret = new Thread(r, "SentryAsyncConnection-" + cnt++);
+    public Thread newThread(final @NotNull Runnable r) {
+      final Thread ret = new Thread(r, "SentryAsyncConnection-" + cnt++);
       ret.setDaemon(true);
       return ret;
     }
   }
 
   private final class EventSender implements Retryable {
-    final SentryEvent event;
+    private final SentryEvent event;
     private final Object hint;
     private final IEventCache eventCache;
-    long suggestedRetryDelay;
-    final TransportResult failedResult = TransportResult.error(5000, -1);
+    private long suggestedRetryDelay;
+    private int responseCode;
+    private final TransportResult failedResult =
+        TransportResult.error(HTTP_RETRY_AFTER_DEFAULT_DELAY_MS, -1);
 
-    EventSender(SentryEvent event, Object hint, IEventCache eventCache) {
+    EventSender(final SentryEvent event, final Object hint, final IEventCache eventCache) {
       this.event = event;
       this.hint = hint;
       this.eventCache = eventCache;
@@ -181,8 +170,9 @@ private TransportResult flush() {
             eventCache.discard(event);
           } else {
             suggestedRetryDelay = result.getRetryMillis();
+            responseCode = result.getResponseCode();
 
-            String message =
+            final String message =
                 "The transport failed to send the event with response code "
                     + result.getResponseCode()
                     + ". Retrying in "
@@ -213,5 +203,10 @@ private TransportResult flush() {
     public long getSuggestedRetryDelayMillis() {
       return suggestedRetryDelay;
     }
+
+    @Override
+    public int getResponseCode() {
+      return responseCode;
+    }
   }
 }

sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java

@@ -1,6 +1,7 @@
 package io.sentry.core.transport;
 
 import static io.sentry.core.SentryLevel.*;
+import static io.sentry.core.transport.RetryingThreadPoolExecutor.HTTP_RETRY_AFTER_DEFAULT_DELAY_MS;
 
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.core.ISerializer;
@@ -50,12 +51,12 @@ public class HttpTransport implements ITransport {
    * @param sentryUrl sentryUrl which is the parsed DSN
    */
   public HttpTransport(
-      SentryOptions options,
-      IConnectionConfigurator connectionConfigurator,
-      int connectionTimeoutMills,
-      int readTimeoutMills,
-      boolean bypassSecurity,
-      URL sentryUrl) {
+      final SentryOptions options,
+      final IConnectionConfigurator connectionConfigurator,
+      final int connectionTimeoutMills,
+      final int readTimeoutMills,
+      final boolean bypassSecurity,
+      final URL sentryUrl) {
     this.proxy = options.getProxy();
     this.connectionConfigurator = connectionConfigurator;
     this.serializer = options.getSerializer();
@@ -68,15 +69,15 @@ public HttpTransport(
 
   // giving up on testing this method is probably the simplest way of having the rest of the class
   // testable...
-  protected HttpURLConnection open(Proxy proxy) throws IOException {
+  protected HttpURLConnection open(final Proxy proxy) throws IOException {
     // why do we need url here? its not used
     return (HttpURLConnection)
         (proxy == null ? sentryUrl.openConnection() : sentryUrl.openConnection(proxy));
   }
 
   @Override
-  public TransportResult send(SentryEvent event) throws IOException {
-    HttpURLConnection connection = open(proxy);
+  public TransportResult send(final SentryEvent event) throws IOException {
+    final HttpURLConnection connection = open(proxy);
     connectionConfigurator.configure(connection);
 
     connection.setRequestMethod("POST");
@@ -97,17 +98,18 @@ public TransportResult send(SentryEvent event) throws IOException {
 
     connection.connect();
 
-    try (OutputStream outputStream = connection.getOutputStream();
-        OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream, UTF_8)) {
+    try (final OutputStream outputStream = connection.getOutputStream();
+        final OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream, UTF_8)) {
       serializer.serialize(event, outputStreamWriter);
 
       // need to also close the input stream of the connection
       connection.getInputStream().close();
       options.getLogger().log(DEBUG, "Event sent %s successfully.", event.getEventId());
       return TransportResult.success();
+      //      throw new IOException();
     } catch (IOException e) {
-      long retryAfterMs = 60000; // the default is 60s
-      String retryAfterHeader = connection.getHeaderField("Retry-After");
+      long retryAfterMs = HTTP_RETRY_AFTER_DEFAULT_DELAY_MS;
+      final String retryAfterHeader = connection.getHeaderField("Retry-After");
       if (retryAfterHeader != null) {
         try {
           retryAfterMs =
@@ -120,6 +122,7 @@ public TransportResult send(SentryEvent event) throws IOException {
       int responseCode = -1;
       try {
         responseCode = connection.getResponseCode();
+
         if (responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
           options
               .getLogger()
@@ -145,7 +148,7 @@ public TransportResult send(SentryEvent event) throws IOException {
     }
   }
 
-  private void logErrorInPayload(HttpURLConnection connection) {
+  private void logErrorInPayload(final HttpURLConnection connection) {
     if (options
         .isDebug()) { // just because its expensive, but internally isDebug is already checked when
       // .log() is called
@@ -162,8 +165,8 @@ private void logErrorInPayload(HttpURLConnection connection) {
     }
   }
 
-  private String getErrorMessageFromStream(InputStream errorStream) {
-    BufferedReader reader = new BufferedReader(new InputStreamReader(errorStream, UTF_8));
+  private String getErrorMessageFromStream(final InputStream errorStream) {
+    final BufferedReader reader = new BufferedReader(new InputStreamReader(errorStream, UTF_8));
     StringBuilder sb = new StringBuilder();
     try {
       String line;

sentry-core/src/main/java/io/sentry/core/transport/Retryable.java

@@ -11,4 +11,6 @@ interface Retryable extends Runnable {
    * delay before the next retry.
    */
   long getSuggestedRetryDelayMillis();
+
+  int getResponseCode();
 }