[RUM-16961] Fix URLSession network tracking when usesClassicLoadingMode is false#3017
[RUM-16961] Fix URLSession network tracking when usesClassicLoadingMode is false#3017mariedm wants to merge 4 commits into
Conversation
mariedm
force-pushed
the
mariedm/RUM-16961/nw-urlsession-task-tracking
branch
12 times, most recently
from
f3c935b to
3ef258f
Compare
mariedm
force-pushed
the
mariedm/RUM-16961/nw-urlsession-task-tracking
branch
from
3ef258f to
d3f1e8a
Compare
| let notifyInterceptionDidStart = expectation(description: "Notify interception did start") | ||
| let notifyInterceptionDidComplete = expectation(description: "Notify interception did complete") | ||
| let server = ServerMock(delivery: .success(response: .mockResponseWith(statusCode: 200), data: .mock(ofSize: 10))) | ||
| let server = ServerMock( |
There was a problem hiding this comment.
The skipIsMainThreadCheck changes are unrelated to this issue but should reduce flakiness.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f94238880
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| intercept(task, error) | ||
| previousImplementation(task, Self.selector, error) |
There was a problem hiding this comment.
When an NWURLSessionTask without a completion handler fails or completes through this hook, this is the only path that calls task(_:didChangeToState:). Because intercept enqueues completion work that reads task.response, task.error, and countOfBytesReceived, running it before previousImplementation lets the SDK finish and remove the interception before Foundation has populated those fields; the same ordering exists in the retryable hook below. That can emit automatic resources missing the error/status/size for usesClassicLoadingMode = false requests, so call the original completion first or pass the completion data through before finishing.
Useful? React with 👍 / 👎.
| defer { lock.unlock() } | ||
| taskSetState = try TaskSetState.build() | ||
| taskSetState?.swizzle(intercept: interceptSetState) | ||
| // NWURLSessionTask bypasses setState:; hook into its completion method instead. |
There was a problem hiding this comment.
It seems there are some use cases where the completion method completeTaskWithError: is not fired:
- NW tasks without completion handlers.
- successful async/await
session.data(from:)
| let session = URLSession(configuration: configuration) | ||
| defer { session.invalidateAndCancel() } | ||
|
|
||
| // When - no completion handler: relies entirely on NWTaskComplete swizzle for end tracking |
There was a problem hiding this comment.
Since url is https://localhost:1, this test only validates the failure path
2 participants
What and why?
On iOS/tvOS 18.4+, setting
URLSessionConfiguration.usesClassicLoadingMode = falseswitches URLSession to a Network.framework-backed task stack that returnsNWURLSessionTaskinstances instead of__NSCFLocalSessionTask. Our existing swizzles only targeted__NSCFLocalSessionTask, so all network tracking silently stopped for sessions using this configuration.Fixes #3012.
How?
Added two inner classes to the existing swizzlers — no changes to public API or the
NetworkInstrumentationFeatureinterface:NWTaskResume(inURLSessionTaskSwizzler): swizzlesNWURLSessionTask.resumealongside the classic task, using the sameinterceptResumecallback.NWTaskComplete(inURLSessionTaskStateSwizzler): swizzlesNWURLSessionTask.completeTaskWithError:(the equivalent ofsetState:on the new stack) and maps it to a completed-state transition.Both return
nilon iOS/tvOS < 18.4 ifNWURLSessionTaskis not present, so there is no impact on existing behavior.Review checklist
make api-surfacewhen adding new APIs — N/A, no new APIs