Implement maximum batch request size (#874)

* Implement maximum request size * Add test when batch exceeds the size * Execute check before gzip * Remove stub request
segmentio · Apr 15, 2020 · 8158687 · 8158687
1 parent 951d057
commit 8158687
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 7 deletions.
diff --git a/Analytics/Classes/Internal/SEGHTTPClient.h b/Analytics/Classes/Internal/SEGHTTPClient.h
@@ -33,7 +33,7 @@ NS_ASSUME_NONNULL_BEGIN
  * NOTE: You need to re-dispatch within the completionHandler onto a desired queue to avoid threading issues.
  * Completion handlers are called on a dispatch queue internal to SEGHTTPClient. 
  */
-- (NSURLSessionUploadTask *)upload:(JSON_DICT)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler;
+- (nullable NSURLSessionUploadTask *)upload:(JSON_DICT)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler;
 
 - (NSURLSessionDataTask *)settingsForWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL success, JSON_DICT _Nullable settings))completionHandler;
 

diff --git a/Analytics/Classes/Internal/SEGHTTPClient.m b/Analytics/Classes/Internal/SEGHTTPClient.m
@@ -2,6 +2,7 @@
 #import "NSData+SEGGZIP.h"
 #import "SEGAnalyticsUtils.h"
 
+static const NSUInteger kMaxBatchSize = 475000; // 475KB
 
 @implementation SEGHTTPClient
 
@@ -66,7 +67,7 @@ - (void)dealloc
 }
 
 
-- (NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler
+- (nullable NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler
 {
     //    batch = SEGCoerceDictionary(batch);
     NSURLSession *session = [self sessionForWriteKey:writeKey];
@@ -93,6 +94,11 @@ - (NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *
         completionHandler(NO); // Don't retry this batch.
         return nil;
     }
+    if (payload.length >= kMaxBatchSize) {
+        SEGLog(@"Payload exceeded the limit of %luKB per batch", kMaxBatchSize / 1000);
+        completionHandler(NO);
+        return nil;
+    }
     NSData *gzippedPayload = [payload seg_gzippedData];
 
     NSURLSessionUploadTask *task = [session uploadTaskWithRequest:request fromData:gzippedPayload completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response, NSError *_Nullable error) {

diff --git a/AnalyticsTests/HTTPClientTest.swift b/AnalyticsTests/HTTPClientTest.swift
@@ -127,7 +127,7 @@ class HTTPClientTest: QuickSpec {
           done = true
         }
         expect(done).toEventually(beTrue())
-        expect(task.state).toEventually(equal(URLSessionTask.State.completed))
+        expect(task?.state).toEventually(equal(URLSessionTask.State.completed))
       }
 
       it("asks to retry for 3xx response") {
@@ -142,7 +142,7 @@ class HTTPClientTest: QuickSpec {
           done = true
         }
         expect(done).toEventually(beTrue())
-        expect(task.state).toEventually(equal(URLSessionTask.State.completed))
+        expect(task?.state).toEventually(equal(URLSessionTask.State.completed))
       }
 
       it("does not ask to retry for 4xx response") {
@@ -157,7 +157,7 @@ class HTTPClientTest: QuickSpec {
           done = true
         }
         expect(done).toEventually(beTrue())
-        expect(task.state).toEventually(equal(URLSessionTask.State.completed))
+        expect(task?.state).toEventually(equal(URLSessionTask.State.completed))
       }
 
       it("asks to retry for 429 response") {
@@ -172,7 +172,7 @@ class HTTPClientTest: QuickSpec {
           done = true
         }
         expect(done).toEventually(beTrue())
-        expect(task.state).toEventually(equal(URLSessionTask.State.completed))
+        expect(task?.state).toEventually(equal(URLSessionTask.State.completed))
       }
 
       it("asks to retry for 5xx response") {
@@ -187,7 +187,19 @@ class HTTPClientTest: QuickSpec {
           done = true
         }
         expect(done).toEventually(beTrue())
-        expect(task.state).toEventually(equal(URLSessionTask.State.completed))
+        expect(task?.state).toEventually(equal(URLSessionTask.State.completed))
+      }
+
+      it("fails when batch size exceeds the max limit size") {
+        let oversizedBatch: [String: Any] = ["sentAt":"2016-07-19'T'19:25:06Z",
+                                             "batch": Array(repeating: ["type":"track", "event":"foo"], count: 16000)]
+        var done = false
+        let task = client.upload(oversizedBatch, forWriteKey: "bar") { retry in
+          expect(retry) == false
+          done = true
+        }
+        expect(done).toEventually(beTrue())
+        expect(task).toEventually(beNil())
       }
     }