-
-
Notifications
You must be signed in to change notification settings - Fork 389
feat(network-details): Introduce new swizzling to capture response bodies for session replay #7584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mobile-935/data-classes
Are you sure you want to change the base?
Changes from all commits
cfbbf21
f685280
5056daf
c24784c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,87 @@ + (void)swizzleURLSessionTask:(SentryNetworkTracker *)networkTracker | |
| #pragma clang diagnostic pop | ||
| } | ||
|
|
||
| /** | ||
| * Swizzles NSURLSession data task creation methods that use completion handlers | ||
| * to enable response body capture for session replay. | ||
| * | ||
| * Both dataTaskWithRequest: and dataTaskWithURL: are independent implementations | ||
| * (neither calls through to the other), so both need swizzling. | ||
| * | ||
| * See SentryNSURLSessionTaskSearchTests that verifies these assumptions still hold. | ||
| */ | ||
| + (void)swizzleURLSessionDataTasksForResponseCapture:(SentryNetworkTracker *)networkTracker | ||
| { | ||
| [self swizzleDataTaskWithRequestCompletionHandler:networkTracker]; | ||
| [self swizzleDataTaskWithURLCompletionHandler:networkTracker]; | ||
| } | ||
|
|
||
| /** | ||
| * Swizzles -[NSURLSession dataTaskWithRequest:completionHandler:] to intercept response data. | ||
| */ | ||
| + (void)swizzleDataTaskWithRequestCompletionHandler:(SentryNetworkTracker *)networkTracker | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
While it is not the same, a strategy similar to SentryNSDataSwizzlingHelperTests.swift could be used to verify the methods are swizzled when executing a datatask The current tests are verifying the method are changed, but not that they are actually called (what we actually care for) Once that is added, this would look good to me to merge
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. something like the following ? (kind of like an API test / integration test - i.e. use the API and validate that the swizzling callbacks a) occur and ideally b) occur as expected ) test scenario
Couple different versions:
Will do. if you see this in time => help me by weighing in on whether this sounds like a test that should use an actual server (there is some test server set-up that i haven't needed to use it but will set it up if you say it's the right direction).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds good to me, you can use https://postman-echo.com
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done - PTAL |
||
| { | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wshadow" | ||
| SEL selector = @selector(dataTaskWithRequest:completionHandler:); | ||
| SentrySwizzleInstanceMethod([NSURLSession class], selector, | ||
| SentrySWReturnType(NSURLSessionDataTask *), | ||
| SentrySWArguments(NSURLRequest * request, | ||
| void (^completionHandler)(NSData *, NSURLResponse *, NSError *)), | ||
| SentrySWReplacement({ | ||
| __block NSURLSessionDataTask *task = nil; | ||
| void (^wrappedHandler)(NSData *, NSURLResponse *, NSError *) = nil; | ||
| if (completionHandler) { | ||
| wrappedHandler = ^(NSData *data, NSURLResponse *response, NSError *error) { | ||
| if (!error && data && task) { | ||
| [networkTracker captureResponseDetails:data | ||
| response:response | ||
| requestURL:request.URL | ||
| task:task]; | ||
| } | ||
| completionHandler(data, response, error); | ||
| }; | ||
| } | ||
| task = SentrySWCallOriginal(request, wrappedHandler ?: completionHandler); | ||
| return task; | ||
43jay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }), | ||
| SentrySwizzleModeOncePerClassAndSuperclasses, (void *)selector); | ||
| #pragma clang diagnostic pop | ||
| } | ||
|
|
||
| /** | ||
| * Swizzles -[NSURLSession dataTaskWithURL:completionHandler:] to intercept response data. | ||
| */ | ||
| + (void)swizzleDataTaskWithURLCompletionHandler:(SentryNetworkTracker *)networkTracker | ||
| { | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wshadow" | ||
| SEL selector = @selector(dataTaskWithURL:completionHandler:); | ||
| SentrySwizzleInstanceMethod([NSURLSession class], selector, | ||
| SentrySWReturnType(NSURLSessionDataTask *), | ||
| SentrySWArguments( | ||
| NSURL * url, void (^completionHandler)(NSData *, NSURLResponse *, NSError *)), | ||
| SentrySWReplacement({ | ||
| __block NSURLSessionDataTask *task = nil; | ||
| void (^wrappedHandler)(NSData *, NSURLResponse *, NSError *) = nil; | ||
| if (completionHandler) { | ||
| wrappedHandler = ^(NSData *data, NSURLResponse *response, NSError *error) { | ||
| if (!error && data && task) { | ||
| [networkTracker captureResponseDetails:data | ||
| response:response | ||
| requestURL:url | ||
| task:task]; | ||
| } | ||
| completionHandler(data, response, error); | ||
| }; | ||
| } | ||
| task = SentrySWCallOriginal(url, wrappedHandler ?: completionHandler); | ||
| return task; | ||
| }), | ||
| SentrySwizzleModeOncePerClassAndSuperclasses, (void *)selector); | ||
| #pragma clang diagnostic pop | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @end | ||
|
|
||
| NS_ASSUME_NONNULL_END | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,80 @@ | ||
| import XCTest | ||
|
|
||
| // We need to know whether Apple changes the NSURLSessionTask implementation. | ||
| class SentryNSURLSessionTaskSearchTests: XCTestCase { | ||
|
|
||
| // We need to know whether Apple changes the NSURLSessionTask implementation. | ||
| func test_URLSessionTask_ByIosVersion() { | ||
| func test_URLSessionTask_ByIosVersion() { | ||
| let classes = SentryNSURLSessionTaskSearch.urlSessionTaskClassesToTrack() | ||
|
|
||
| XCTAssertEqual(classes.count, 1) | ||
| XCTAssertTrue(classes.first === URLSessionTask.self) | ||
| } | ||
|
|
||
| // MARK: - NSURLSession class hierarchy validation tests | ||
| // | ||
| // Based on testing, NSURLSession implements dataTaskWithRequest:completionHandler: | ||
| // and dataTaskWithURL:completionHandler: directly on the base class. | ||
| // | ||
| // The swizzling code relies on this by swizzling [NSURLSession class] directly | ||
| // rather than doing runtime discovery. These tests verify that assumption | ||
| // still holds β if Apple ever moves these methods, these tests | ||
| // will fail and we'll know to update the swizzling approach. | ||
|
|
||
| func test_URLSessionDataTaskWithRequest_ByIosVersion() { | ||
| let selector = #selector(URLSession.dataTask(with:completionHandler:) | ||
| as (URLSession) -> (URLRequest, @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask) | ||
| assertNSURLSessionImplementsDirectly(selector: selector, selectorName: "dataTaskWithRequest:completionHandler:") | ||
| } | ||
43jay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| func test_URLSessionDataTaskWithURL_ByIosVersion() { | ||
| let selector = #selector(URLSession.dataTask(with:completionHandler:) | ||
| as (URLSession) -> (URL, @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask) | ||
| assertNSURLSessionImplementsDirectly(selector: selector, selectorName: "dataTaskWithURL:completionHandler:") | ||
| } | ||
|
|
||
| // MARK: - Helper | ||
|
|
||
| /// Walks the class hierarchy for sessions created with default and ephemeral | ||
| /// configurations and asserts that no subclass overrides `selector`. | ||
| private func assertNSURLSessionImplementsDirectly(selector: Selector, selectorName: String) { | ||
| let baseClass: AnyClass = URLSession.self | ||
|
|
||
| // The base class must implement the method. | ||
| XCTAssertNotNil( | ||
| class_getInstanceMethod(baseClass, selector), | ||
| "URLSession should implement \(selectorName)" | ||
| ) | ||
|
|
||
| // Check sessions created with each relevant configuration. | ||
| let configs: [URLSessionConfiguration] = [ | ||
| .default, | ||
| .ephemeral | ||
| ] | ||
|
|
||
| for config in configs { | ||
| let session = URLSession(configuration: config) | ||
| let sessionClass: AnyClass = type(of: session) | ||
|
|
||
| defer { session.invalidateAndCancel() } | ||
|
|
||
| if sessionClass === baseClass { | ||
| continue | ||
| } | ||
|
|
||
| // If Apple returns a subclass, it must NOT provide its own | ||
| // implementation β it should inherit from URLSession. | ||
| let subMethod = class_getInstanceMethod(sessionClass, selector) | ||
| let baseMethod = class_getInstanceMethod(baseClass, selector) | ||
|
|
||
| if let subMethod, let baseMethod { | ||
| let subIMP = method_getImplementation(subMethod) | ||
| let baseIMP = method_getImplementation(baseMethod) | ||
| XCTAssertEqual( | ||
| subIMP, baseIMP, | ||
| "\(NSStringFromClass(sessionClass)) overrides \(selectorName) with an unexpected IMP β " | ||
| + "Verify swizzling in SentrySwizzleWrapperHelper is correct for dataTasks." | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| #if os(iOS) || os(tvOS) | ||
|
Check failure on line 1 in Tests/SentryTests/Integrations/Performance/Network/SentryNetworkDetailSwizzlingTests.swift
|
||
|
|
||
| @_spi(Private) @testable import Sentry | ||
| @_spi(Private) import SentryTestUtils | ||
| import XCTest | ||
|
|
||
| /// Integration tests that verify the completion-handler swizzling for replay's | ||
| /// network detail capture actually works end-to-end. | ||
| /// | ||
| /// Unlike the unit tests in SentryNetworkTrackerTests (which call tracker | ||
| /// methods directly), these tests start the SDK, make real HTTP requests, | ||
| /// and assert that the swizzled completion handler fires and populates | ||
| /// network details on the resulting breadcrumb. | ||
| /// | ||
| /// Uses postman-echo.com so no local test server is required. | ||
| class SentryNetworkDetailSwizzlingTests: XCTestCase { | ||
|
|
||
| private let echoURL = URL(string: "https://postman-echo.com/get")! | ||
|
|
||
| override func setUp() { | ||
| super.setUp() | ||
|
|
||
| let options = Options() | ||
| options.dsn = TestConstants.dsnAsString(username: "SentryNetworkDetailSwizzlingTests") | ||
| options.tracesSampleRate = 1.0 | ||
| options.enableNetworkBreadcrumbs = true | ||
| options.sessionReplay.networkDetailAllowUrls = ["postman-echo.com"] | ||
| options.sessionReplay.networkCaptureBodies = true | ||
| SentrySDK.start(options: options) | ||
| } | ||
|
|
||
| override func tearDown() { | ||
| super.tearDown() | ||
| clearTestState() | ||
| } | ||
|
|
||
| // MARK: - Tests | ||
|
|
||
| /// Verifies the swizzle of `-[NSURLSession dataTaskWithRequest:completionHandler:]` | ||
| /// captures response details into the breadcrumb. | ||
| func testDataTaskWithRequest_completionHandler_capturesNetworkDetails() throws { | ||
| let transaction = SentrySDK.startTransaction( | ||
| name: "Test", operation: "test", bindToScope: true | ||
| ) | ||
|
|
||
| let expect = expectation(description: "Request completed") | ||
| expect.assertForOverFulfill = false | ||
|
|
||
| let session = URLSession(configuration: .default) | ||
| let request = URLRequest(url: echoURL) | ||
|
|
||
| var receivedData: Data? | ||
| var receivedResponse: URLResponse? | ||
| var receivedError: Error? | ||
|
|
||
| let task = session.dataTask(with: request) { data, response, error in | ||
| receivedData = data | ||
| receivedResponse = response | ||
| receivedError = error | ||
| expect.fulfill() | ||
| } | ||
| defer { task.cancel() } | ||
|
|
||
| task.resume() | ||
| wait(for: [expect], timeout: 5) | ||
|
|
||
| transaction.finish() | ||
|
|
||
| // Original completion handler received valid data | ||
| XCTAssertNil(receivedError, "Request should succeed") | ||
| XCTAssertNotNil(receivedData, "Should receive response data") | ||
| let httpResponse = try XCTUnwrap(receivedResponse as? HTTPURLResponse) | ||
| XCTAssertEqual(httpResponse.statusCode, 200) | ||
|
|
||
| // Network details were captured via the swizzled completion handler | ||
| let breadcrumb = try lastHTTPBreadcrumb(for: echoURL) | ||
| let details = try XCTUnwrap( | ||
|
Check failure on line 77 in Tests/SentryTests/Integrations/Performance/Network/SentryNetworkDetailSwizzlingTests.swift
|
||
| breadcrumb.data?[SentryReplayNetworkDetails.replayNetworkDetailsKey] as? SentryReplayNetworkDetails, | ||
| "Swizzled completion handler should have populated network details on the breadcrumb" | ||
| ) | ||
| let serialized = details.serialize() | ||
| XCTAssertEqual(serialized["statusCode"] as? Int, 200) | ||
| XCTAssertNotNil(serialized["response"], "Response details should be captured") | ||
| } | ||
|
|
||
| /// Verifies the swizzle of `-[NSURLSession dataTaskWithURL:completionHandler:]` | ||
| /// captures response details into the breadcrumb. | ||
| func testDataTaskWithURL_completionHandler_capturesNetworkDetails() throws { | ||
| let transaction = SentrySDK.startTransaction( | ||
| name: "Test", operation: "test", bindToScope: true | ||
| ) | ||
|
|
||
| let expect = expectation(description: "Request completed") | ||
| expect.assertForOverFulfill = false | ||
|
|
||
| let session = URLSession(configuration: .default) | ||
|
|
||
| var receivedData: Data? | ||
| var receivedResponse: URLResponse? | ||
| var receivedError: Error? | ||
|
|
||
| let task = session.dataTask(with: echoURL) { data, response, error in | ||
| receivedData = data | ||
| receivedResponse = response | ||
| receivedError = error | ||
| expect.fulfill() | ||
| } | ||
| defer { task.cancel() } | ||
|
|
||
| task.resume() | ||
| wait(for: [expect], timeout: 5) | ||
|
|
||
| transaction.finish() | ||
|
|
||
| // Original completion handler received valid data | ||
| XCTAssertNil(receivedError, "Request should succeed") | ||
| XCTAssertNotNil(receivedData, "Should receive response data") | ||
| let httpResponse = try XCTUnwrap(receivedResponse as? HTTPURLResponse) | ||
| XCTAssertEqual(httpResponse.statusCode, 200) | ||
|
|
||
| // Network details were captured via the swizzled completion handler | ||
| let breadcrumb = try lastHTTPBreadcrumb(for: echoURL) | ||
| let details = try XCTUnwrap( | ||
|
Check failure on line 123 in Tests/SentryTests/Integrations/Performance/Network/SentryNetworkDetailSwizzlingTests.swift
|
||
| breadcrumb.data?[SentryReplayNetworkDetails.replayNetworkDetailsKey] as? SentryReplayNetworkDetails, | ||
| "Swizzled completion handler should have populated network details on the breadcrumb" | ||
| ) | ||
| let serialized = details.serialize() | ||
| XCTAssertEqual(serialized["statusCode"] as? Int, 200) | ||
| XCTAssertNotNil(serialized["response"], "Response details should be captured") | ||
| } | ||
|
|
||
| // MARK: - Helpers | ||
|
|
||
| /// Finds the most recent HTTP breadcrumb whose URL matches the given URL. | ||
| private func lastHTTPBreadcrumb(for url: URL) throws -> Breadcrumb { | ||
| let scope = SentrySDKInternal.currentHub().scope | ||
| let breadcrumbs = try XCTUnwrap( | ||
| Dynamic(scope).breadcrumbArray as [Breadcrumb]?, | ||
| "Scope should contain breadcrumbs" | ||
| ) | ||
| let matching = breadcrumbs.filter { | ||
| $0.category == "http" && ($0.data?["url"] as? String)?.contains(url.host ?? "") == true | ||
| } | ||
| return try XCTUnwrap(matching.last, "Should find an HTTP breadcrumb for \(url)") | ||
| } | ||
| } | ||
|
|
||
| #endif // os(iOS) || os(tvOS) | ||
Uh oh!
There was an error while loading. Please reload this page.