Skip to content

Commit 04459bd

Browse files
tapworktapwork
authored
Clean Instana errors (#12)
* Clean up InstanaError - Convert to type Error - Clean up codes - Improve tests * Remove beacon from queue if http client error occured
1 parent a0806fd commit 04459bd

File tree

13 files changed

+231
-64
lines changed

13 files changed

+231
-64
lines changed

‎Sources/InstanaAgent/Beacons/CoreBeacon/CoreBeaconFactory.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class CoreBeaconFactory {
3131
let message = "Beacon <-> CoreBeacon mapping for beacon \(beacon) not defined"
3232
debugAssertFailure(message)
3333
session.logger.add(message, level: .error)
34-
throw InstanaError(code: .unknownType, description: message)
34+
throw InstanaError.unknownType(message)
3535
}
3636
return cbeacon
3737
}

‎Sources/InstanaAgent/Beacons/Reporter.swift‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,13 @@ public class Reporter {
106106
func flushQueue() {
107107
let connectionType = networkUtility.connectionType
108108
guard connectionType != .none else {
109-
return complete([], .failure(InstanaError(code: .offline, description: "No connection available")))
109+
return complete([], .failure(InstanaError.offline))
110110
}
111111
if suspendReporting.contains(.cellularConnection), connectionType == .cellular {
112-
return complete([], .failure(InstanaError(code: .noWifiAvailable, description: "No WIFI Available")))
112+
return complete([], .failure(InstanaError.noWifiAvailable))
113113
}
114114
if suspendReporting.contains(.lowBattery), !batterySafeForNetworking() {
115-
return complete([], .failure(InstanaError(code: .lowBattery, description: "Battery too low for flushing")))
115+
return complete([], .failure(InstanaError.lowBattery))
116116
}
117117

118118
let beacons = queue.items
@@ -130,10 +130,8 @@ public class Reporter {
130130
switch result {
131131
case let .failure(error):
132132
self.complete(beacons, .failure(error))
133-
case .success(200 ... 299):
133+
case .success:
134134
self.complete(beacons, .success)
135-
case let .success(statusCode):
136-
self.complete(beacons, .failure(InstanaError(code: .invalidResponse, description: "Invalid repsonse status code: \(statusCode)")))
137135
}
138136
}
139137
}
@@ -154,8 +152,8 @@ public class Reporter {
154152
beaconsToBeRemoved = beacons
155153
case let .failure(error):
156154
session.logger.add("Failed to send Beacon batch: \(error)", level: .warning)
157-
// Beacons will be removed here if queue is full, otherwise the beacons will be kept for the next flush
158-
beaconsToBeRemoved = queue.isFull ? beacons : []
155+
let removeFromQueue = queue.isFull || (error as? InstanaError).isHTTPClientError
156+
beaconsToBeRemoved = removeFromQueue ? beacons : []
159157
}
160158
queue.remove(beaconsToBeRemoved) { [weak self] _ in
161159
guard let self = self else { return }
@@ -168,7 +166,7 @@ public class Reporter {
168166
extension Reporter {
169167
func createBatchRequest(from beacons: String) throws -> URLRequest {
170168
guard !session.configuration.key.isEmpty else {
171-
throw InstanaError(code: .notAuthenticated, description: "Missing application key. No data will be sent.")
169+
throw InstanaError.missingAppKey
172170
}
173171

174172
var urlRequest = URLRequest(url: session.configuration.reportingURL)

‎Sources/InstanaAgent/Error/InstanaError.swift‎

Lines changed: 87 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,97 @@
11
import Foundation
22

33
/// Represents errors that can be thrown by the Instana SDK
4-
class InstanaError: NSError {
5-
static let domain = "com.instana.ios.agent.error"
4+
enum InstanaError: LocalizedError, Equatable {
5+
static func == (lhs: InstanaError, rhs: InstanaError) -> Bool {
6+
lhs as NSError == rhs as NSError
7+
}
8+
9+
case fileHandling(String)
10+
case invalidRequest
11+
case httpClientError(Int)
12+
case httpServerError(Int)
13+
case invalidResponse
14+
case missingAppKey
15+
case unknownType(String)
16+
case noWifiAvailable
17+
case offline
18+
case lowBattery
19+
case underlying(Error)
20+
21+
var localizedDescription: String {
22+
switch self {
23+
case let .fileHandling(value):
24+
return "File handling failed \(value)"
25+
case .invalidRequest:
26+
return "Invalid URLRequest"
27+
case let .httpClientError(code):
28+
return "HTTP Client error occured code: \(code)"
29+
case let .httpServerError(code):
30+
return "HTTP Server error occured code: \(code)"
31+
case .invalidResponse:
32+
return "Invalid response type"
33+
case .missingAppKey:
34+
return "Missing Instana app key"
35+
case let .unknownType(value):
36+
return "Type mismatch \(value)"
37+
case .noWifiAvailable:
38+
return "No WIFI Available"
39+
case .offline:
40+
return "No Internet connection available"
41+
case .lowBattery:
42+
return "Battery too low for flushing"
43+
case let .underlying(error):
44+
return "Underlying error \(error)"
45+
}
46+
}
47+
48+
var errorDescription: String? {
49+
localizedDescription
50+
}
651

7-
enum Code: Int {
8-
case invalidRequest
9-
case invalidURL
10-
case invalidResponse
11-
case notAuthenticated
12-
case bufferOverwrite
13-
case unknownType
14-
case noWifiAvailable
15-
case offline
16-
case lowBattery
17-
case instanaInstanceNotFound
52+
var isHTTPClientError: Bool {
53+
switch self {
54+
case .httpClientError:
55+
return true
56+
default:
57+
return false
58+
}
1859
}
1960

20-
init(code: Code, description: String) {
21-
super.init(domain: InstanaError.domain, code: code.rawValue, userInfo: [NSLocalizedDescriptionKey: description])
61+
var isUnknownType: Bool {
62+
switch self {
63+
case .unknownType:
64+
return true
65+
default:
66+
return false
67+
}
68+
}
69+
70+
static func create(from error: Error) -> InstanaError {
71+
let nserror = error as NSError
72+
if nserror.code == NSURLErrorNotConnectedToInternet {
73+
return InstanaError.offline
74+
}
75+
return InstanaError.underlying(error)
76+
}
77+
}
78+
79+
extension Optional where Wrapped == InstanaError {
80+
var isHTTPClientError: Bool {
81+
switch self {
82+
case let .some(value):
83+
return value.isHTTPClientError
84+
default:
85+
return false
86+
}
2287
}
2388

24-
required init?(coder aDecoder: NSCoder) {
25-
super.init(coder: aDecoder)
89+
var isUnknownType: Bool {
90+
switch self {
91+
case let .some(value):
92+
return value.isUnknownType
93+
default:
94+
return false
95+
}
2696
}
2797
}

‎Sources/InstanaAgent/Monitors/HTTP/HTTPMonitor.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class HTTPMonitor {
3939
extension HTTPMonitor {
4040
func mark(_ request: URLRequest) throws -> HTTPMarker {
4141
guard let url = request.url, let method = request.httpMethod else {
42-
throw InstanaError(code: InstanaError.Code.invalidRequest, description: "Invalid URLRequest")
42+
throw InstanaError.invalidRequest
4343
}
4444
return HTTPMarker(url: url, method: method, trigger: .automatic, delegate: self)
4545
}

‎Sources/InstanaAgent/Utils/InstanaPersistableQueue.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class InstanaPersistableQueue<T: Codable & Equatable> {
6262

6363
func deserialize() throws -> [T] {
6464
guard let fileURL = queueJSONFileURL else {
65-
throw InstanaError(code: InstanaError.Code.invalidRequest, description: "Cache path not found")
65+
throw InstanaError.fileHandling("Cache path not found")
6666
}
6767
let data = try Data(contentsOf: fileURL)
6868
return try JSONDecoder().decode([T].self, from: data)

‎Sources/InstanaAgent/Utils/Networking/InstanaNetworking.swift‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,20 @@ class InstanaNetworking {
1818
func send(request: URLRequest, completion: @escaping LoadResult) {
1919
send(request) { _, response, error in
2020
if let error = error {
21-
completion(.failure(error))
22-
} else if let httpResponse = response as? HTTPURLResponse {
21+
return completion(.failure(InstanaError.create(from: error)))
22+
}
23+
guard let httpResponse = response as? HTTPURLResponse else {
24+
return completion(.failure(InstanaError.invalidResponse))
25+
}
26+
switch httpResponse.statusCode {
27+
case 200 ... 399:
2328
completion(.success(statusCode: httpResponse.statusCode))
24-
} else {
25-
completion(.failure(InstanaError(code: .invalidResponse, description: "Unexpected response type")))
29+
case 400 ... 499:
30+
completion(.failure(InstanaError.httpClientError(httpResponse.statusCode)))
31+
case 500 ... 599:
32+
completion(.failure(InstanaError.httpServerError(httpResponse.statusCode)))
33+
default:
34+
completion(.failure(InstanaError.invalidResponse))
2635
}
2736
}.resume()
2837
}

‎Tests/InstanaAgentTests/Beacons/CoreBeacon/CoreBeaconFactoryTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class CoreBeaconFactoryTests: InstanaTestCase {
1515
// When
1616
XCTAssertThrowsError(try factory.map(beacon)) {error in
1717
// Then
18-
XCTAssertEqual((error as? InstanaError)?.code, InstanaError.Code.unknownType.rawValue)
18+
AssertTrue((error as? InstanaError).isUnknownType)
1919
}
2020
}
2121

‎Tests/InstanaAgentTests/Beacons/ReporterTests.swift‎

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ class ReporterTests: InstanaTestCase {
224224

225225
// Then
226226
AssertTrue(sendNotCalled)
227-
AssertTrue(resultError?.code == InstanaError.Code.offline.rawValue)
227+
AssertTrue(resultError == InstanaError.offline)
228228
}
229229

230230
/// Send when coming back offline again (starting offline
@@ -259,7 +259,7 @@ class ReporterTests: InstanaTestCase {
259259

260260
// Then
261261
AssertTrue(sendCalled == false)
262-
AssertTrue(resultError?.code == InstanaError.Code.offline.rawValue)
262+
AssertTrue(resultError == InstanaError.offline)
263263

264264
// When coming back online
265265
networkUtility.update(.wifi)
@@ -299,7 +299,7 @@ class ReporterTests: InstanaTestCase {
299299

300300
// Then
301301
AssertTrue(sendNotCalled)
302-
AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
302+
AssertTrue(resultError == InstanaError.noWifiAvailable)
303303
}
304304

305305
/// Criteria:
@@ -379,7 +379,7 @@ class ReporterTests: InstanaTestCase {
379379

380380
// Then
381381
AssertTrue(sendNotCalled)
382-
AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
382+
AssertTrue(resultError == InstanaError.noWifiAvailable)
383383
}
384384

385385
// MARK: Test suspending behavior on LOW Battery
@@ -410,7 +410,7 @@ class ReporterTests: InstanaTestCase {
410410

411411
// Then
412412
AssertTrue(sendNotCalled)
413-
AssertTrue(resultError?.code == InstanaError.Code.lowBattery.rawValue)
413+
AssertTrue(resultError == InstanaError.lowBattery)
414414
}
415415

416416
/// Criteria:
@@ -490,7 +490,7 @@ class ReporterTests: InstanaTestCase {
490490

491491
// Then
492492
AssertTrue(didNOTSendReport)
493-
AssertTrue(resultError?.code == InstanaError.Code.lowBattery.rawValue)
493+
AssertTrue(resultError == InstanaError.lowBattery)
494494
}
495495

496496
// MARK: Test suspending behavior on all (NO WIFI and low Battery)
@@ -521,7 +521,7 @@ class ReporterTests: InstanaTestCase {
521521

522522
// Then
523523
AssertTrue(sendNotCalled)
524-
AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
524+
AssertTrue(resultError == InstanaError.noWifiAvailable)
525525
}
526526

527527
/// Criteria:
@@ -550,7 +550,7 @@ class ReporterTests: InstanaTestCase {
550550

551551
// Then
552552
AssertTrue(sendNotCalled)
553-
AssertTrue(resultError?.code == InstanaError.Code.noWifiAvailable.rawValue)
553+
AssertTrue(resultError == InstanaError.noWifiAvailable)
554554
}
555555

556556
/// Criteria:
@@ -579,7 +579,7 @@ class ReporterTests: InstanaTestCase {
579579

580580
// Then
581581
AssertTrue(sendNotCalled)
582-
AssertTrue(resultError?.code == InstanaError.Code.lowBattery.rawValue)
582+
AssertTrue(resultError == InstanaError.lowBattery)
583583
}
584584

585585
/// Criteria:
@@ -890,6 +890,56 @@ class ReporterTests: InstanaTestCase {
890890
AssertEqualAndNotNil(reporter.queue.items.last?.bid, givenBeacon.id.uuidString)
891891
}
892892

893+
func test_remove_from_after_http_client_error() {
894+
// Given
895+
let beacon = HTTPBeacon.createMock()
896+
let mockQueue = MockInstanaPersistableQueue<CoreBeacon>(identifier: "", maxItems: 2)
897+
let givenError = InstanaError.httpClientError(400)
898+
let waitForSend = expectation(description: "Delayed sending")
899+
let reporter = Reporter(session(), batterySafeForNetworking: { true }, networkUtility: .wifi, queue: mockQueue) { _, completion in
900+
DispatchQueue.main.async {
901+
completion(.failure(givenError))
902+
}
903+
}
904+
905+
// When
906+
reporter.completionHandler.append {result in
907+
waitForSend.fulfill()
908+
}
909+
reporter.submit(beacon)
910+
wait(for: [waitForSend], timeout: 2.0)
911+
912+
// Then
913+
AssertTrue(reporter.queue.items.isEmpty)
914+
AssertTrue(mockQueue.removedItems.count == 1)
915+
AssertTrue(mockQueue.removedItems.first?.bid == beacon.id.uuidString)
916+
}
917+
918+
func test_remove_from_after_queue_full() {
919+
// Given
920+
let beacon = HTTPBeacon.createMock()
921+
let mockQueue = MockInstanaPersistableQueue<CoreBeacon>(identifier: "", maxItems: 1)
922+
let givenError = InstanaError.httpServerError(500)
923+
let waitForSend = expectation(description: "Delayed sending")
924+
let reporter = Reporter(session(), batterySafeForNetworking: { true }, networkUtility: .wifi, queue: mockQueue) { _, completion in
925+
DispatchQueue.main.async {
926+
completion(.failure(givenError))
927+
}
928+
}
929+
930+
// When
931+
reporter.completionHandler.append {result in
932+
waitForSend.fulfill()
933+
}
934+
reporter.submit(beacon)
935+
wait(for: [waitForSend], timeout: 2.0)
936+
937+
// Then
938+
AssertTrue(reporter.queue.items.isEmpty)
939+
AssertTrue(mockQueue.removedItems.count == 1)
940+
AssertTrue(mockQueue.removedItems.first?.bid == beacon.id.uuidString)
941+
}
942+
893943
func test_invalid_beacon_should_not_submitted() {
894944
// Given
895945
var shouldNotSubmitted = true
@@ -949,7 +999,7 @@ class ReporterTests: InstanaTestCase {
949999
wait(for: [waitForSend], timeout: 10.0)
9501000

9511001
// Then
952-
AssertEqualAndNotZero(resultError?.code ?? 0, InstanaError.Code.invalidResponse.rawValue)
1002+
AssertTrue(resultError == InstanaError.invalidResponse)
9531003
}
9541004

9551005
func test_submit_and_flush_shouldNotCause_RetainCycle() {
@@ -980,7 +1030,7 @@ class ReporterTests: InstanaTestCase {
9801030
let waitFor = expectation(description: "Wait For")
9811031
let reporter = Reporter(session(), batterySafeForNetworking: { true }, networkUtility: .wifi) { _, completion in
9821032
DispatchQueue.main.async {
983-
completion(.failure(InstanaError(code: .invalidResponse, description: "SomeError")))
1033+
completion(.failure(InstanaError.invalidResponse))
9841034
}
9851035
}
9861036
let beacons: [HTTPBeacon] = (0..<reporter.queue.maxItems).map { _ in HTTPBeacon.createMock() }
@@ -1038,7 +1088,7 @@ extension ReporterTests {
10381088
// When
10391089
XCTAssertThrowsError(try reporter.createBatchRequest(from: corebeacons.asString)) {error in
10401090
// Then
1041-
XCTAssertEqual((error as? InstanaError)?.code, InstanaError.Code.notAuthenticated.rawValue)
1091+
XCTAssertEqual((error as? InstanaError), InstanaError.missingAppKey)
10421092
}
10431093
}
10441094
}

‎Tests/InstanaAgentTests/IntegrationTest/InstanaIntegrationTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ class InstanaIntegrationTests: InstanaTestCase {
148148
let name = "Some name"
149149
let duration: Instana.Types.Milliseconds = 12
150150
let backendTracingID = "BackendID"
151-
let error: NSError = InstanaError(code: .invalidResponse, description: "Some")
151+
let error = InstanaError.invalidResponse
152152
let mKey = "Key"
153153
let mValue = "Value"
154154
let viewName = "View"

0 commit comments

Comments
 (0)