Skip to content

Commit

Permalink
track statusCode along with error message for failed http request (#51)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hongyan Jiang authored and GitHub Enterprise committed Jul 16, 2024
1 parent 2647310 commit 023bb11
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 18 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions Sources/InstanaAgent/Error/HTTPError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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"
}
Expand Down Expand Up @@ -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)"
}
Expand All @@ -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 {
Expand Down
9 changes: 5 additions & 4 deletions Sources/InstanaAgent/Monitors/HTTP/HTTPMarker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = [:]
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
6 changes: 3 additions & 3 deletions Tests/InstanaAgentTests/Error/HTTPErrorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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")
}

Expand Down
11 changes: 6 additions & 5 deletions Tests/InstanaAgentTests/Monitors/HTTP/HTTPMarkerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down Expand Up @@ -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)")
Expand Down Expand Up @@ -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
}
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 023bb11

Please sign in to comment.