if retry after header has empty categories, apply retry after to all …

…of them (#377)
getsentry · Apr 27, 2020 · 2e84810 · 2e84810
1 parent 403e196
commit 2e84810
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 7 deletions.
diff --git a/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java b/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java
@@ -57,6 +57,7 @@ public class HttpTransport implements ITransport {
   private final @NotNull Map<String, Date> sentryRetryAfterLimit = new ConcurrentHashMap<>();
 
   private static final int HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS = 60000;
+  private static final String HTTP_RETRY_DEFAULT_CATEGORY = "default";
 
   /**
    * Constructs a new HTTP transport instance. Notably, the provided {@code requestUpdater} must set
@@ -164,8 +165,8 @@ public boolean isRetryAfter(final @NotNull String type) {
       final Date date = sentryRetryAfterLimit.get(type);
 
       return !new Date().after(date);
-    } else if (sentryRetryAfterLimit.containsKey("default")) {
-      final Date date = sentryRetryAfterLimit.get("default");
+    } else if (sentryRetryAfterLimit.containsKey(HTTP_RETRY_DEFAULT_CATEGORY)) {
+      final Date date = sentryRetryAfterLimit.get(HTTP_RETRY_DEFAULT_CATEGORY);
 
       return !new Date().after(date);
     }
@@ -260,7 +261,15 @@ public boolean isRetryAfter(final @NotNull String type) {
 
   private void updateRetryAfterLimits(
       final @NotNull HttpURLConnection connection, final int responseCode) {
+    // seconds
     final String retryAfterHeader = connection.getHeaderField("Retry-After");
+
+    // X-Sentry-Rate-Limits looks like: seconds:categories:scope
+    // it could have more than one scope so it looks like:
+    // quota_limit, quota_limit, quota_limit
+
+    // a real example: 50:transaction:key, 2700:default;event;security:organization
+    // 50::key is also a valid case, it means no categories and it should apply to all of them
     final String sentryRateLimitHeader = connection.getHeaderField("X-Sentry-Rate-Limits");
     updateRetryAfterLimits(sentryRateLimitHeader, retryAfterHeader, responseCode);
   }
@@ -292,13 +301,23 @@ private void updateRetryAfterLimits(
           if (retryAfterAndCategories.length > 1) {
             final String allCategories = retryAfterAndCategories[1];
 
-            if (allCategories != null) {
+            // we dont care if Date is UTC as we just add the relative seconds
+            final Date date = new Date(System.currentTimeMillis() + retryAfterMillis);
+
+            if (allCategories != null && !allCategories.isEmpty()) {
               final String[] categories = allCategories.split(";", -1);
 
               for (final String catItem : categories) {
-                // we dont care if Date is UTC as we just add the relative seconds
-                sentryRetryAfterLimit.put(
-                    catItem, new Date(System.currentTimeMillis() + retryAfterMillis));
+                sentryRetryAfterLimit.put(catItem, date);
+              }
+            } else {
+              // if categories are empty, we should apply to all the categories.
+              for (final String catItem : sentryRetryAfterLimit.keySet()) {
+                sentryRetryAfterLimit.put(catItem, date);
+              }
+              // if 'default' category is not added yet, we add it as a fallback
+              if (!sentryRetryAfterLimit.containsKey(HTTP_RETRY_DEFAULT_CATEGORY)) {
+                sentryRetryAfterLimit.put(HTTP_RETRY_DEFAULT_CATEGORY, date);
               }
             }
           }
@@ -308,7 +327,7 @@ private void updateRetryAfterLimits(
       final long retryAfterMillis = parseRetryAfterOrDefault(retryAfterHeader);
       // we dont care if Date is UTC as we just add the relative seconds
       final Date date = new Date(System.currentTimeMillis() + retryAfterMillis);
-      sentryRetryAfterLimit.put("default", date);
+      sentryRetryAfterLimit.put(HTTP_RETRY_DEFAULT_CATEGORY, date);
     }
   }
 

diff --git a/sentry-core/src/test/java/io/sentry/core/transport/HttpTransportTest.kt b/sentry-core/src/test/java/io/sentry/core/transport/HttpTransportTest.kt
@@ -282,6 +282,21 @@ class HttpTransportTest {
         assertFalse(transport.isRetryAfter("security"))
     }
 
+    @Test
+    fun `When X-Sentry-Rate-Limit categories are empty, applies to all the categories`() {
+        val transport = fixture.getSUT()
+
+        whenever(fixture.connection.inputStream).thenThrow(IOException())
+        whenever(fixture.connection.getHeaderField(eq("X-Sentry-Rate-Limits")))
+            .thenReturn("50::key")
+
+        val event = SentryEvent()
+
+        transport.send(event)
+
+        assertTrue(transport.isRetryAfter("event"))
+    }
+
     private fun createSession(): Session {
         return Session("123", User(), "env", "release")
     }