Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Sources/Sentry/SentryNetworkTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -562,4 +562,15 @@ - (SentryLevel)getBreadcrumbLevel:(NSURLSessionTask *)sessionTask
return breadcrumbLevel;
}

- (void)captureResponseDetails:(NSData *)data
response:(NSURLResponse *)response
requestURL:(NSURL *)requestURL
task:(NSURLSessionTask *)task
{
// TODO: Implementation
// 2. Parse response body data
// 3. Store in appropriate location for session replay
// 4. Handle size limits and truncation if needed
}
Comment thread
43jay marked this conversation as resolved.

@end
81 changes: 81 additions & 0 deletions Sources/Sentry/SentrySwizzleWrapperHelper.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h: I would like to see some test verifying the swizzling actually work.
While this looks good, it may break at any point in later versions.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The 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

  1. Initialize a NSUrlSessionDataTask
  2. resume it (or whatever the API is to get it to run)
  3. have it run to completion
  4. verify our completionHandler called, verify the original completion handler called

Couple different versions:
For both newly swizzled initializers:

  1. With networkDetailAllowUrls populated (feature turned on; swizzling active)
  2. With networkDetailAllowUrls populated (feature turned off; swizzling non-existent)

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).
Or if the current test harness (Γ  la unit test with mocking) is good enough,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good to me, you can use https://postman-echo.com
If it is flaky we can boot a local server, but I would prefer not to do that if possible

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The 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;
Comment thread
43jay marked this conversation as 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
}
Comment thread
cursor[bot] marked this conversation as resolved.

@end

NS_ASSUME_NONNULL_END
5 changes: 5 additions & 0 deletions Sources/Sentry/include/SentryNetworkTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ static NSString *const SENTRY_NETWORK_REQUEST_TRACKER_BREADCRUMB
@property (nonatomic, readonly) BOOL isCaptureFailedRequestsEnabled;
@property (nonatomic, readonly) BOOL isGraphQLOperationTrackingEnabled;

- (void)captureResponseDetails:(NSData *)data
response:(NSURLResponse *)response
requestURL:(nullable NSURL *)requestURL
task:(NSURLSessionTask *)task;

@end

NS_ASSUME_NONNULL_END
4 changes: 4 additions & 0 deletions Sources/Sentry/include/SentrySwizzleWrapperHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ NS_ASSUME_NONNULL_BEGIN

+ (void)swizzleURLSessionTask:(SentryNetworkTracker *)networkTracker;

// Swizzle [NSURLSession dataTaskWithURL:completionHandler:]
// [NSURLSession dataTaskWithRequest:completionHandler:]
+ (void)swizzleURLSessionDataTasksForResponseCapture:(SentryNetworkTracker *)networkTracker;

@end

NS_ASSUME_NONNULL_END
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ final class SentryNetworkTrackingIntegration<Dependencies: NetworkTrackerProvide
super.init()

SentrySwizzleWrapperHelper.swizzleURLSessionTask(networkTracker)

Comment thread
43jay marked this conversation as resolved.
#if os(iOS) || os(tvOS)
if options.sessionReplay.networkDetailHasUrls {
SentrySwizzleWrapperHelper.swizzleURLSessionDataTasks(forResponseCapture: networkTracker)
}
#endif
}

func uninstall() {
Expand Down
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:")
}
Comment thread
43jay marked this conversation as 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

View workflow job for this annotation

GitHub Actions / JUnit Test Report

SentryNetworkDetailSwizzlingTests.testDataTaskWithRequest_completionHandler_capturesNetworkDetails

/Users/admin/actions-runner/_work/sentry-cocoa/sentry-cocoa/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkDetailSwizzlingTests.swift:77 - XCTUnwrap failed: expected non-nil value of type "SentryReplayNetworkDetails" - Swizzled completion handler should have populated network details on the breadcrumb

Check failure on line 1 in Tests/SentryTests/Integrations/Performance/Network/SentryNetworkDetailSwizzlingTests.swift

View workflow job for this annotation

GitHub Actions / JUnit Test Report

SentryNetworkDetailSwizzlingTests.testDataTaskWithURL_completionHandler_capturesNetworkDetails

/Users/admin/actions-runner/_work/sentry-cocoa/sentry-cocoa/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkDetailSwizzlingTests.swift:123 - XCTUnwrap failed: expected non-nil value of type "SentryReplayNetworkDetails" - Swizzled completion handler should have populated network details on the breadcrumb

@_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

View workflow job for this annotation

GitHub Actions / Fast Unit Tests (iOS 18) / Unit iOS 18 Sentry

testDataTaskWithRequest_completionHandler_capturesNetworkDetails, XCTUnwrap failed: expected non-nil value of type "SentryReplayNetworkDetails" - Swizzled completion handler should have populated network details on the breadcrumb
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

View workflow job for this annotation

GitHub Actions / Fast Unit Tests (iOS 18) / Unit iOS 18 Sentry

testDataTaskWithURL_completionHandler_capturesNetworkDetails, XCTUnwrap failed: expected non-nil value of type "SentryReplayNetworkDetails" - Swizzled completion handler should have populated network details on the breadcrumb
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)
Loading