Clean Instana errors

Sources/InstanaAgent/Beacons/CoreBeacon/CoreBeaconFactory.swift

@@ -31,7 +31,7 @@ class CoreBeaconFactory {
             let message = "Beacon <-> CoreBeacon mapping for beacon \(beacon) not defined"
             debugAssertFailure(message)
             session.logger.add(message, level: .error)
-            throw InstanaError(code: .unknownType, description: message)
+            throw InstanaError.unknownType(message)
         }
         return cbeacon
     }

Sources/InstanaAgent/Beacons/Reporter.swift

@@ -106,13 +106,13 @@ public class Reporter {
     func flushQueue() {
         let connectionType = networkUtility.connectionType
         guard connectionType != .none else {
-            return complete([], .failure(InstanaError(code: .offline, description: "No connection available")))
+            return complete([], .failure(InstanaError.offline))
         }
         if suspendReporting.contains(.cellularConnection), connectionType == .cellular {
-            return complete([], .failure(InstanaError(code: .noWifiAvailable, description: "No WIFI Available")))
+            return complete([], .failure(InstanaError.noWifiAvailable))
         }
         if suspendReporting.contains(.lowBattery), !batterySafeForNetworking() {
-            return complete([], .failure(InstanaError(code: .lowBattery, description: "Battery too low for flushing")))
+            return complete([], .failure(InstanaError.lowBattery))
         }
 
         let beacons = queue.items
@@ -130,10 +130,8 @@ public class Reporter {
             switch result {
             case let .failure(error):
                 self.complete(beacons, .failure(error))
-            case .success(200 ... 299):
+            case .success:
                 self.complete(beacons, .success)
-            case let .success(statusCode):
-                self.complete(beacons, .failure(InstanaError(code: .invalidResponse, description: "Invalid repsonse status code: \(statusCode)")))
             }
         }
     }
@@ -154,8 +152,8 @@ public class Reporter {
             beaconsToBeRemoved = beacons
         case let .failure(error):
             session.logger.add("Failed to send Beacon batch: \(error)", level: .warning)
-            // Beacons will be removed here if queue is full, otherwise the beacons will be kept for the next flush
-            beaconsToBeRemoved = queue.isFull ? beacons : []
+            let removeFromQueue = queue.isFull || (error as? InstanaError).isHTTPClientError
+            beaconsToBeRemoved = removeFromQueue ? beacons : []
         }
         queue.remove(beaconsToBeRemoved) { [weak self] _ in
             guard let self = self else { return }
@@ -168,7 +166,7 @@ public class Reporter {
 extension Reporter {
     func createBatchRequest(from beacons: String) throws -> URLRequest {
         guard !session.configuration.key.isEmpty else {
-            throw InstanaError(code: .notAuthenticated, description: "Missing application key. No data will be sent.")
+            throw InstanaError.missingAppKey
         }
 
         var urlRequest = URLRequest(url: session.configuration.reportingURL)

Sources/InstanaAgent/Error/InstanaError.swift

@@ -1,27 +1,97 @@
 import Foundation
 
 /// Represents errors that can be thrown by the Instana SDK
-class InstanaError: NSError {
-    static let domain = "com.instana.ios.agent.error"
+enum InstanaError: LocalizedError, Equatable {
+    static func == (lhs: InstanaError, rhs: InstanaError) -> Bool {
+        lhs as NSError == rhs as NSError
+    }
+
+    case fileHandling(String)
+    case invalidRequest
+    case httpClientError(Int)
+    case httpServerError(Int)
+    case invalidResponse
+    case missingAppKey
+    case unknownType(String)
+    case noWifiAvailable
+    case offline
+    case lowBattery
+    case underlying(Error)
+
+    var localizedDescription: String {
+        switch self {
+        case let .fileHandling(value):
+            return "File handling failed \(value)"
+        case .invalidRequest:
+            return "Invalid URLRequest"
+        case let .httpClientError(code):
+            return "HTTP Client error occured code: \(code)"
+        case let .httpServerError(code):
+            return "HTTP Server  error occured code: \(code)"
+        case .invalidResponse:
+            return "Invalid response type"
+        case .missingAppKey:
+            return "Missing Instana app key"
+        case let .unknownType(value):
+            return "Type mismatch \(value)"
+        case .noWifiAvailable:
+            return "No WIFI Available"
+        case .offline:
+            return "No Internet connection available"
+        case .lowBattery:
+            return "Battery too low for flushing"
+        case let .underlying(error):
+            return "Underlying error \(error)"
+        }
+    }
+
+    var errorDescription: String? {
+        localizedDescription
+    }
 
-    enum Code: Int {
-        case invalidRequest
-        case invalidURL
-        case invalidResponse
-        case notAuthenticated
-        case bufferOverwrite
-        case unknownType
-        case noWifiAvailable
-        case offline
-        case lowBattery
-        case instanaInstanceNotFound
+    var isHTTPClientError: Bool {
+        switch self {
+        case .httpClientError:
+            return true
+        default:
+            return false
+        }
     }
 
-    init(code: Code, description: String) {
-        super.init(domain: InstanaError.domain, code: code.rawValue, userInfo: [NSLocalizedDescriptionKey: description])
+    var isUnknownType: Bool {
+        switch self {
+        case .unknownType:
+            return true
+        default:
+            return false
+        }
+    }
+
+    static func create(from error: Error) -> InstanaError {
+        let nserror = error as NSError
+        if nserror.code == NSURLErrorNotConnectedToInternet {
+            return InstanaError.offline
+        }
+        return InstanaError.underlying(error)
+    }
+}
+
+extension Optional where Wrapped == InstanaError {
+    var isHTTPClientError: Bool {
+        switch self {
+        case let .some(value):
+            return value.isHTTPClientError
+        default:
+            return false
+        }
     }
 
-    required init?(coder aDecoder: NSCoder) {
-        super.init(coder: aDecoder)
+    var isUnknownType: Bool {
+        switch self {
+        case let .some(value):
+            return value.isUnknownType
+        default:
+            return false
+        }
     }
 }

Sources/InstanaAgent/Monitors/HTTP/HTTPMonitor.swift

@@ -39,7 +39,7 @@ class HTTPMonitor {
 extension HTTPMonitor {
     func mark(_ request: URLRequest) throws -> HTTPMarker {
         guard let url = request.url, let method = request.httpMethod else {
-            throw InstanaError(code: InstanaError.Code.invalidRequest, description: "Invalid URLRequest")
+            throw InstanaError.invalidRequest
         }
         return HTTPMarker(url: url, method: method, trigger: .automatic, delegate: self)
     }

Sources/InstanaAgent/Utils/InstanaPersistableQueue.swift

@@ -62,7 +62,7 @@ class InstanaPersistableQueue<T: Codable & Equatable> {
 
     func deserialize() throws -> [T] {
         guard let fileURL = queueJSONFileURL else {
-            throw InstanaError(code: InstanaError.Code.invalidRequest, description: "Cache path not found")
+            throw InstanaError.fileHandling("Cache path not found")
         }
         let data = try Data(contentsOf: fileURL)
         return try JSONDecoder().decode([T].self, from: data)

Sources/InstanaAgent/Utils/Networking/InstanaNetworking.swift

@@ -18,11 +18,20 @@ class InstanaNetworking {
     func send(request: URLRequest, completion: @escaping LoadResult) {
         send(request) { _, response, error in
             if let error = error {
-                completion(.failure(error))
-            } else if let httpResponse = response as? HTTPURLResponse {
+                return completion(.failure(InstanaError.create(from: error)))
+            }
+            guard let httpResponse = response as? HTTPURLResponse else {
+                return completion(.failure(InstanaError.invalidResponse))
+            }
+            switch httpResponse.statusCode {
+            case 200 ... 399:
                 completion(.success(statusCode: httpResponse.statusCode))
-            } else {
-                completion(.failure(InstanaError(code: .invalidResponse, description: "Unexpected response type")))
+            case 400 ... 499:
+                completion(.failure(InstanaError.httpClientError(httpResponse.statusCode)))
+            case 500 ... 599:
+                completion(.failure(InstanaError.httpServerError(httpResponse.statusCode)))
+            default:
+                completion(.failure(InstanaError.invalidResponse))
             }
         }.resume()
     }

Tests/InstanaAgentTests/Beacons/CoreBeacon/CoreBeaconFactoryTests.swift

@@ -15,7 +15,7 @@ class CoreBeaconFactoryTests: InstanaTestCase {
         // When
         XCTAssertThrowsError(try factory.map(beacon)) {error in
             // Then
-            XCTAssertEqual((error as? InstanaError)?.code, InstanaError.Code.unknownType.rawValue)
+            AssertTrue((error as? InstanaError).isUnknownType)
         }
     }
 

Tests/InstanaAgentTests/Beacons/ReporterTests.swift

@@ -224,7 +224,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.offline.rawValue)
+        AssertTrue(resultError == InstanaError.offline)
     }
 
     /// Send when coming back offline again (starting offline
@@ -259,7 +259,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendCalled == false)
-        AssertTrue(resultError?.code == InstanaError.Code.offline.rawValue)
+        AssertTrue(resultError == InstanaError.offline)
 
         // When coming back online
         networkUtility.update(.wifi)
@@ -299,7 +299,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
+        AssertTrue(resultError == InstanaError.noWifiAvailable)
     }
 
     /// Criteria:
@@ -379,7 +379,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
+        AssertTrue(resultError == InstanaError.noWifiAvailable)
     }
 
     // MARK: Test suspending behavior on LOW Battery
@@ -410,7 +410,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.lowBattery.rawValue)
+        AssertTrue(resultError == InstanaError.lowBattery)
     }
 
     /// Criteria:
@@ -490,7 +490,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(didNOTSendReport)
-        AssertTrue(resultError?.code == InstanaError.Code.lowBattery.rawValue)
+        AssertTrue(resultError == InstanaError.lowBattery)
     }
 
     // MARK: Test suspending behavior on all (NO WIFI and low Battery)
@@ -521,7 +521,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
+        AssertTrue(resultError == InstanaError.noWifiAvailable)
     }
 
     /// Criteria:
@@ -550,7 +550,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
+        AssertTrue(resultError == InstanaError.noWifiAvailable)
     }
 
     /// Criteria:
@@ -579,7 +579,7 @@ class ReporterTests: InstanaTestCase {
 
         // Then
         AssertTrue(sendNotCalled)
-        AssertTrue(resultError?.code == InstanaError.Code.lowBattery.rawValue)
+        AssertTrue(resultError == InstanaError.lowBattery)
     }
 
     /// Criteria:
@@ -890,6 +890,56 @@ class ReporterTests: InstanaTestCase {
         AssertEqualAndNotNil(reporter.queue.items.last?.bid, givenBeacon.id.uuidString)
     }
 
+    func test_remove_from_after_http_client_error() {
+        // Given
+        let beacon = HTTPBeacon.createMock()
+        let mockQueue = MockInstanaPersistableQueue<CoreBeacon>(identifier: "", maxItems: 2)
+        let givenError = InstanaError.httpClientError(400)
+        let waitForSend = expectation(description: "Delayed sending")
+        let reporter = Reporter(session(), batterySafeForNetworking: { true }, networkUtility: .wifi, queue: mockQueue) { _, completion in
+            DispatchQueue.main.async {
+                completion(.failure(givenError))
+            }
+        }
+
+        // When
+        reporter.completionHandler.append {result in
+            waitForSend.fulfill()
+        }
+        reporter.submit(beacon)
+        wait(for: [waitForSend], timeout: 2.0)
+
+        // Then
+        AssertTrue(reporter.queue.items.isEmpty)
+        AssertTrue(mockQueue.removedItems.count == 1)
+        AssertTrue(mockQueue.removedItems.first?.bid == beacon.id.uuidString)
+    }
+
+    func test_remove_from_after_queue_full() {
+        // Given
+        let beacon = HTTPBeacon.createMock()
+        let mockQueue = MockInstanaPersistableQueue<CoreBeacon>(identifier: "", maxItems: 1)
+        let givenError = InstanaError.httpServerError(500)
+        let waitForSend = expectation(description: "Delayed sending")
+        let reporter = Reporter(session(), batterySafeForNetworking: { true }, networkUtility: .wifi, queue: mockQueue) { _, completion in
+            DispatchQueue.main.async {
+                completion(.failure(givenError))
+            }
+        }
+
+        // When
+        reporter.completionHandler.append {result in
+            waitForSend.fulfill()
+        }
+        reporter.submit(beacon)
+        wait(for: [waitForSend], timeout: 2.0)
+
+        // Then
+        AssertTrue(reporter.queue.items.isEmpty)
+        AssertTrue(mockQueue.removedItems.count == 1)
+        AssertTrue(mockQueue.removedItems.first?.bid == beacon.id.uuidString)
+    }
+
     func test_invalid_beacon_should_not_submitted() {
         // Given
         var shouldNotSubmitted = true
@@ -949,7 +999,7 @@ class ReporterTests: InstanaTestCase {
         wait(for: [waitForSend], timeout: 10.0)
 
         // Then
-        AssertEqualAndNotZero(resultError?.code ?? 0, InstanaError.Code.invalidResponse.rawValue)
+        AssertTrue(resultError == InstanaError.invalidResponse)
     }
 
     func test_submit_and_flush_shouldNotCause_RetainCycle() {
@@ -980,7 +1030,7 @@ class ReporterTests: InstanaTestCase {
         let waitFor = expectation(description: "Wait For")
         let reporter = Reporter(session(), batterySafeForNetworking: { true }, networkUtility: .wifi) { _, completion in
             DispatchQueue.main.async {
-                completion(.failure(InstanaError(code: .invalidResponse, description: "SomeError")))
+                completion(.failure(InstanaError.invalidResponse))
             }
         }
         let beacons: [HTTPBeacon] = (0..<reporter.queue.maxItems).map { _ in HTTPBeacon.createMock() }
@@ -1038,7 +1088,7 @@ extension ReporterTests {
         // When
         XCTAssertThrowsError(try reporter.createBatchRequest(from: corebeacons.asString)) {error in
             // Then
-            XCTAssertEqual((error as? InstanaError)?.code, InstanaError.Code.notAuthenticated.rawValue)
+            XCTAssertEqual((error as? InstanaError), InstanaError.missingAppKey)
         }
     }
 }

Tests/InstanaAgentTests/IntegrationTest/InstanaIntegrationTests.swift

@@ -148,7 +148,7 @@ class InstanaIntegrationTests: InstanaTestCase {
         let name = "Some name"
         let duration = "12"
         let backendTracingID = "BackendID"
-        let error: NSError = InstanaError(code: .invalidResponse, description: "Some")
+        let error = InstanaError.invalidResponse
         let mKey = "Key"
         let mValue = "Value"
         let viewName = "View"