From 023bb11db23b0add4d43b2da3a28b7d7620eb5eb Mon Sep 17 00:00:00 2001 From: Hongyan Jiang Date: Mon, 15 Jul 2024 19:08:25 -0700 Subject: [PATCH] track statusCode along with error message for failed http request (#51) --- Changelog.md | 1 + Sources/InstanaAgent/Error/HTTPError.swift | 14 ++++++++++---- .../InstanaAgent/Monitors/HTTP/HTTPMarker.swift | 9 +++++---- .../Beacons/Beacon Types/HTTPBeaconTests.swift | 3 ++- Tests/InstanaAgentTests/Error/HTTPErrorTests.swift | 6 +++--- .../Monitors/HTTP/HTTPMarkerTests.swift | 11 ++++++----- .../Monitors/HTTP/InstanaURLProtocolTests.swift | 3 ++- 7 files changed, 29 insertions(+), 18 deletions(-) diff --git a/Changelog.md b/Changelog.md index b6ba47d..926d8b5 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ ## 1.8.2 - crash beacon errorType value update +- When http request failed, track statusCode along with error message ## 1.8.1 - added functionality to accept internal metadata from cross-platform agents diff --git a/Sources/InstanaAgent/Error/HTTPError.swift b/Sources/InstanaAgent/Error/HTTPError.swift index 5f6b3c5..8dd6a09 100644 --- a/Sources/InstanaAgent/Error/HTTPError.swift +++ b/Sources/InstanaAgent/Error/HTTPError.swift @@ -43,7 +43,7 @@ enum HTTPError: LocalizedError, RawRepresentable, CustomStringConvertible, Equat case userAuthenticationRequired case userCancelledAuthentication - case statusCode(Int) + case statusCode(Int, NSError?) case unknownHTTPError(NSError) case unknown(NSError) @@ -84,7 +84,7 @@ enum HTTPError: LocalizedError, RawRepresentable, CustomStringConvertible, Equat case .unsupportedURL: return "Unsupported URL" case .userAuthenticationRequired: return "User Authentication Required" case .userCancelledAuthentication: return "User Cancelled Authentication" - case let .statusCode(code): return "HTTP \(code)" + case let .statusCode(code, _): return "HTTP \(code)" case .unknownHTTPError: return "URL Error" case .unknown: return "Error" } @@ -135,7 +135,13 @@ enum HTTPError: LocalizedError, RawRepresentable, CustomStringConvertible, Equat case .unsupportedURL: return "A properly formed URL couldn’t be handled by the framework." case .userAuthenticationRequired: return "Authentication was required to access a resource." case .userCancelledAuthentication: return "An asynchronous request for authentication has been canceled by the user." - case let .statusCode(code): return "HTTP Error with status code \(code)" + + case let .statusCode(code, nsError): + guard let error = nsError else { + return "HTTP Error with status code \(code)" + } + return error.localizedDescription + case let .unknownHTTPError(error): return "\(error.localizedDescription)" case let .unknown(error): return "\(error.localizedDescription)" } @@ -149,7 +155,7 @@ enum HTTPError: LocalizedError, RawRepresentable, CustomStringConvertible, Equat // swiftlint:disable:next cyclomatic_complexity init?(error: NSError?, statusCode: Int? = nil) { if let httpCode = statusCode, 400 ... 599 ~= httpCode { - self = .statusCode(httpCode) + self = .statusCode(httpCode, error) return } guard let error = error else { diff --git a/Sources/InstanaAgent/Monitors/HTTP/HTTPMarker.swift b/Sources/InstanaAgent/Monitors/HTTP/HTTPMarker.swift index c1bd0d1..bed00ee 100644 --- a/Sources/InstanaAgent/Monitors/HTTP/HTTPMarker.swift +++ b/Sources/InstanaAgent/Monitors/HTTP/HTTPMarker.swift @@ -31,7 +31,7 @@ protocol HTTPMarkerDelegate: AnyObject { @objc public class HTTPMarker: NSObject { enum State { - case started, failed(error: Error), finished(responseCode: Int), canceled + case started, failed(responseCode: Int, error: Error), finished(responseCode: Int), canceled } enum Trigger { @@ -85,7 +85,7 @@ protocol HTTPMarkerDelegate: AnyObject { /// Note: Make sure you don't call any methods on this HTTPMarker after you called finish @objc public func finish(response: URLResponse?, error: Error?) { let httpURLResponse = (response as? HTTPURLResponse) - let statusCode = httpURLResponse?.statusCode ?? 400 + let statusCode = httpURLResponse?.statusCode ?? -1 let size = response != nil ? HTTPMarker.Size(response!) : nil var bothHeaders: HTTPHeader = [:] @@ -124,7 +124,7 @@ protocol HTTPMarkerDelegate: AnyObject { @objc public func finish(_ result: HTTPCaptureResult) { guard case .started = state else { return } if let error = result.error { - state = .failed(error: error) + state = .failed(responseCode: result.statusCode, error: error) } else { state = .finished(responseCode: result.statusCode) } @@ -168,7 +168,8 @@ extension HTTPMarker { error = NSError(domain: NSURLErrorDomain, code: NSURLErrorCancelled, userInfo: nil) case let .finished(code): responseCode = code - case let .failed(theError): + case let .failed(code, theError): + responseCode = code error = theError } let header = filter.filterHeaderFields(header) diff --git a/Tests/InstanaAgentTests/Beacons/Beacon Types/HTTPBeaconTests.swift b/Tests/InstanaAgentTests/Beacons/Beacon Types/HTTPBeaconTests.swift index 8fee9cc..57da678 100644 --- a/Tests/InstanaAgentTests/Beacons/Beacon Types/HTTPBeaconTests.swift +++ b/Tests/InstanaAgentTests/Beacons/Beacon Types/HTTPBeaconTests.swift @@ -116,7 +116,8 @@ class HTTPBeaconTests: InstanaTestCase { AssertEqualAndNotNil(sut.hs, String(code)) AssertTrue(sut.ec == "1") AssertTrue(sut.et == "HTTPError") - AssertTrue(sut.em == "HTTP \(code): HTTP Error with status code \(code)") + let expectedPrefix = "HTTP \(code): " + AssertTrue(sut.em!.starts(with: expectedPrefix)) } } diff --git a/Tests/InstanaAgentTests/Error/HTTPErrorTests.swift b/Tests/InstanaAgentTests/Error/HTTPErrorTests.swift index bcb6184..8dcbca8 100644 --- a/Tests/InstanaAgentTests/Error/HTTPErrorTests.swift +++ b/Tests/InstanaAgentTests/Error/HTTPErrorTests.swift @@ -363,7 +363,7 @@ class HTTPErrorTests: InstanaTestCase { let sut = HTTPError(error: nil, statusCode: 404) // Then - AssertEqualAndNotNil(sut, HTTPError.statusCode(404)) + AssertEqualAndNotNil(sut, HTTPError.statusCode(404, nil)) AssertEqualAndNotNil(sut?.description, "HTTP Error with status code 404") AssertEqualAndNotNil(sut?.rawValue, "HTTP 404") } @@ -373,8 +373,8 @@ class HTTPErrorTests: InstanaTestCase { let sut = HTTPError(error: error(NSURLErrorTimedOut), statusCode: 404) // Then - AssertEqualAndNotNil(sut, HTTPError.statusCode(404)) - AssertEqualAndNotNil(sut?.description, "HTTP Error with status code 404") + AssertEqualAndNotNil(sut, HTTPError.statusCode(404, nil)) + AssertEqualAndNotNil(sut?.description, sut?.localizedDescription) AssertEqualAndNotNil(sut?.rawValue, "HTTP 404") } diff --git a/Tests/InstanaAgentTests/Monitors/HTTP/HTTPMarkerTests.swift b/Tests/InstanaAgentTests/Monitors/HTTP/HTTPMarkerTests.swift index c94dcf2..a82b6b3 100644 --- a/Tests/InstanaAgentTests/Monitors/HTTP/HTTPMarkerTests.swift +++ b/Tests/InstanaAgentTests/Monitors/HTTP/HTTPMarkerTests.swift @@ -176,7 +176,7 @@ class HTTPMarkerTests: InstanaTestCase { XCTAssertEqual(marker.trigger, .manual) XCTAssertEqual(delegate.didFinishCount, 1) XCTAssertEqual(marker.backendTracingID, "BackendID") - if case let .failed(error: result) = marker.state { + if case let .failed(_, error: result) = marker.state { XCTAssertEqual(result as? InstanaError, expectedError) } else { XCTFail("Wrong marker state: \(marker.state)") @@ -247,7 +247,7 @@ class HTTPMarkerTests: InstanaTestCase { XCTAssertEqual(delegate.didFinishCount, 1) XCTAssertEqual(marker.responseSize, responseSize) XCTAssertTrue(marker.duration > 0) - if case let .failed(e) = marker.state { + if case let .failed(_, e) = marker.state { XCTAssertEqual(e as? CocoaError, error) } else { XCTFail("Wrong marker state: \(marker.state)") @@ -345,9 +345,10 @@ class HTTPMarkerTests: InstanaTestCase { let marker = HTTPMarker(url: url, method: "t", trigger: .automatic, delegate: Delegate()) let error = NSError(domain: NSCocoaErrorDomain, code: -1, userInfo: nil) + let statusCode = 409 // When marker.set(responseSize: responseSize) - marker.finish(response: createMockResponse(409), error: error) + marker.finish(response: createMockResponse(statusCode), error: error) guard let beacon = marker.createBeacon(filter: .init()) as? HTTPBeacon else { XCTFail("Beacon type missmatch"); return } @@ -358,12 +359,12 @@ class HTTPMarkerTests: InstanaTestCase { XCTAssertEqual(beacon.duration, marker.duration) XCTAssertEqual(beacon.method, "t") XCTAssertEqual(beacon.url, url) - XCTAssertEqual(beacon.responseCode, -1) + XCTAssertEqual(beacon.responseCode, statusCode) AssertEqualAndNotNil(beacon.responseSize, responseSize) AssertEqualAndNotNil(beacon.responseSize?.headerBytes, responseSize.headerBytes) AssertEqualAndNotNil(beacon.responseSize?.bodyBytes, responseSize.bodyBytes) AssertEqualAndNotNil(beacon.responseSize?.bodyBytesAfterDecoding, responseSize.bodyBytesAfterDecoding) - XCTAssertEqual(beacon.error, HTTPError.unknown(error)) + XCTAssertEqual(beacon.error?.description, error.localizedDescription) } func test_createBeacon_canceledMarker() { diff --git a/Tests/InstanaAgentTests/Monitors/HTTP/InstanaURLProtocolTests.swift b/Tests/InstanaAgentTests/Monitors/HTTP/InstanaURLProtocolTests.swift index 1ad1987..7ea35fa 100644 --- a/Tests/InstanaAgentTests/Monitors/HTTP/InstanaURLProtocolTests.swift +++ b/Tests/InstanaAgentTests/Monitors/HTTP/InstanaURLProtocolTests.swift @@ -356,7 +356,8 @@ class InstanaURLProtocolTests: InstanaTestCase { wait(for: [waitFor], timeout: 3.0) AssertEqualAndNotNil(urlProtocol.marker?.backendTracingID, backendTracingID) AssertTrue(delegate.calledFinalized) - if case let .failed(error) = urlProtocol.marker?.state { + let responseCode = 400 + if case let .failed(responseCode, error) = urlProtocol.marker?.state { resultError = error as NSError } else { XCTFail("Wrong state for marker")