Breadcrumb overflow (#119)

* Breadcrumb overflow

* Version update

* Fix a problem with invalid breadcrumb file size

* Add support for remove subrange

* Updated changelog

Author: konraddysput

Backtrace.podspec

@@ -9,7 +9,7 @@
 Pod::Spec.new do |s|
 
   s.name         = "Backtrace"
-  s.version      = "2.0.1"
+  s.version      = "2.0.2"
   s.swift_version = '5'
   s.summary      = "Backtrace's integration with iOS, macOS and tvOS"
   s.description  = "Reliable crash and hang reporting for iOS, macOS and tvOS."

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Backtrace Cocoa Release Notes
 
+## Version 2.0.2
+- Fixed infinity loop generated by the breadcrumb overflow
+- Adjusted device.model attribute - now the attribute shows device model, rather than the model id
+
 ## Version 2.0.1
 - Added application.session and application.version attribute as defaults - no matter if the metrics integration is enabled or not.
 - Added application.build attribute that represents an app build version.

Sources/Features/Attributes/DefaultAttributes.swift

@@ -101,7 +101,7 @@ struct Device: AttributesSource {
     var immutable: [String: Any?] {
         return [
             "device.machine": try? System.machine(),
-            "device.model": try? System.model(),
+            "device.model": try? System.machine(),
             "uname.sysname": getSysname()
         ]
     }
@@ -207,7 +207,7 @@ struct LibInfo: AttributesSource {
     private static let applicationGuidKey = "backtrace.unique.user.identifier"
     private static let applicationLangName = "backtrace-cocoa"
 
-    var backtraceVersion = "2.0.1"
+    var backtraceVersion = "2.0.2"
 
     var immutable: [String: Any?] {
         return ["guid": LibInfo.guid(store: UserDefaultsStore.self).uuidString,

Sources/Features/Breadcrumb/BacktraceBreadcrumbFile.swift

@@ -17,14 +17,7 @@ enum BacktraceBreadcrumbFileError: Error {
         self.breadcrumbLogURL = try breadcrumbSettings.getBreadcrumbLogPath()
         self.queue = Queue<BreadcrumbRecord>()
         self.maximumIndividualBreadcrumbSize = breadcrumbSettings.maxIndividualBreadcrumbSizeBytes
-        if breadcrumbSettings.maxQueueFileSizeBytes < BacktraceBreadcrumbFile.minimumQueueFileSizeBytes {
-            BacktraceLogger.warning("\(breadcrumbSettings.maxQueueFileSizeBytes) is smaller than the minimum of " +
-                                    "\(BacktraceBreadcrumbFile.minimumQueueFileSizeBytes)" +
-                                    ", ignoring value and overriding with minimum.")
-            self.maxQueueFileSizeBytes = BacktraceBreadcrumbFile.minimumQueueFileSizeBytes
-        } else {
-            self.maxQueueFileSizeBytes = breadcrumbSettings.maxQueueFileSizeBytes
-        }
+        self.maxQueueFileSizeBytes = breadcrumbSettings.maxQueueFileSizeBytes
 
         super.init()
     }
@@ -60,9 +53,7 @@ enum BacktraceBreadcrumbFileError: Error {
                 let breadcrumbSize = queueBreadcrumb.size
                 // Pop last element if size is greater than maxQueueFileSizeBytes
                 if size + breadcrumbSize > maxQueueFileSizeBytes && !queue.isEmpty {
-                    while (index != 0) {
-                        _ = queue.pop(at: index)
-                    }
+                    queue.removeSubrange(range: (0...index))
                     break
                 }
                 let breadcrumbJsonData = queueBreadcrumb.json

Sources/Features/Breadcrumb/BacktraceBreadcrumbs.swift

@@ -10,7 +10,7 @@ import Foundation
     case user = 6
     case configuration = 7
 
-    var description: String {
+    public var description: String {
         switch self {
         case .manual:
             return "manual"
@@ -42,7 +42,7 @@ import Foundation
     case error = 4
     case fatal = 5
 
-    var description: String {
+    public var description: String {
         switch self {
         case .debug:
             return "debug"
@@ -101,7 +101,7 @@ import Foundation
         BreadcrumbsInfo.currentBreadcrumbsId = nil
     }
 
-    func addBreadcrumb(_ message: String,
+    public func addBreadcrumb(_ message: String,
                        attributes: [String: String]? = nil,
                        type: BacktraceBreadcrumbType = BacktraceBreadcrumbType.manual,
                        level: BacktraceBreadcrumbLevel = BacktraceBreadcrumbLevel.info) -> Bool {
@@ -123,7 +123,7 @@ import Foundation
         return breadcrumbsLogManager?.clear() ?? false
     }
 
-    var getCurrentBreadcrumbId: Int? {
+    public var getCurrentBreadcrumbId: Int? {
         return breadcrumbsLogManager?.getCurrentBreadcrumbId
     }
 }

Sources/Features/Breadcrumb/Queue.swift

@@ -20,10 +20,18 @@ public class Queue<T>: NSObject {
         return elements.first
     }
 
+    func removeSubrange(range: ClosedRange<Int>) {
+        elements.removeSubrange(range);
+    }
+    
     func remove(at index: Int) -> T? {
         guard index < elements.count else {
             return nil
         }
+        
+        if (index < 0) {
+            return nil
+        }
         return elements.remove(at: index)
     }
 

Tests/BacktraceBreadcrumbTests.swift

@@ -45,7 +45,7 @@ final class BacktraceBreadcrumbTests: QuickSpec {
                 }
             }
             context("when constructed") {
-                it("Clear clears the file") {
+                it("Clears the file") {
                     expect { manager?.addBreadcrumb("this is a Breadcrumb",
                                                     attributes: nil,
                                                     type: BacktraceBreadcrumbType.system,
@@ -151,6 +151,37 @@ final class BacktraceBreadcrumbTests: QuickSpec {
                 }
             }
             context("rollover and async tests") {
+                it("should remove old breadcrumb and add a new one") {
+                    let settings = BacktraceBreadcrumbSettings()
+                    let maximumNumberOfBreadcrumbs = 4
+                    let breadcrumbMessage = "this is test"
+                    let breadcrumbLevel = BacktraceBreadcrumbLevel.debug
+                    let breadcrumbType =  BacktraceBreadcrumbType.log
+                    let breadcrumb: [String: Any] = ["timestamp":  Date().millisecondsSince1970,
+                                                     "id": 1,
+                                                     "level": breadcrumbLevel.description,
+                                                     "type": breadcrumbType.description,
+                                                     "message": breadcrumbMessage]
+                    
+                    let breadcrumbJsonData = try JSONSerialization.data(withJSONObject: breadcrumb)
+                    let breadcrumbJsonString = String(data: breadcrumbJsonData, encoding: .utf8)
+                    let breadcrumbSize = breadcrumbJsonString!.count
+                    
+                    
+                    settings.maxQueueFileSizeBytes = breadcrumbSize * maximumNumberOfBreadcrumbs + maximumNumberOfBreadcrumbs
+                    breadcrumbs.enableBreadcrumbs(settings)
+                    
+                    for index in (0...maximumNumberOfBreadcrumbs) {
+                        _ = breadcrumbs.addBreadcrumb("\(breadcrumbMessage)\(index)", type: breadcrumbType, level: breadcrumbLevel)
+                    }
+                    
+                    // expect to clean up the file
+                    _ = breadcrumbs.addBreadcrumb("\(breadcrumbMessage)cleanup", type: breadcrumbType, level: breadcrumbLevel)
+                    let breadcrumbText = self.readBreadcrumbText()
+                    expect { breadcrumbText }.notTo(contain("\(breadcrumbMessage)0"))
+                    expect { breadcrumbText }.to(contain("\(breadcrumbMessage)cleanup"))
+                }
+                
                 it("rolls over after enough breadcrumbs are added to get to the maximum file size") {
                     let settings = BacktraceBreadcrumbSettings()
                     settings.maxQueueFileSizeBytes = 32 * 1024