chore: sync upstream PR #8546 - fix(ios) Canceling a WKURLSchemeTask cancels the associated URLSessionTask - #119
chore: sync upstream PR #8546 - fix(ios) Canceling a WKURLSchemeTask cancels the associated URLSessionTask#119riderx wants to merge 1 commit into
Conversation
|
Git applied the upstream-preferred strategy to resolve this sync. Please review the branch carefully before merging. |
Beta npm buildMaintainers can publish one Capacitor Plus workspace package from this PR to npm for fast testing. Comment Examples: /publish-beta core
/publish-beta cli
/publish-beta @capacitor-plus/coreIf exactly one workspace package changed, Packages:
The workflow will:
Security note: beta publish is only enabled for branches inside this repository. |
|
Warning Review limit reached
On-demand reviews are free for the next 24 days. After that, they cost $0.25 per reviewed file. Or wait 8 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 55 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 36 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
e9d5a4b to
c6a9a35
Compare
c6a9a35 to
c1e6786
Compare
|
Git applied the upstream-preferred strategy to resolve this sync. Please review the branch carefully before merging. |
Right now, when WebKit cancels a request (due to navigating to a new page, client JS manually cancelling a fetch via AbortController, etc) it calls the handler's webView(_:stop:). Today that only sets a "stopped" flag on the WKURLSchemeTask (an associated bool added via an extension) but doesn't actually stop the underlying network request. In other words: cancelling a network request using native browser semantics does not actually cancel said network request. In my own app this causes a production issue. We cancel requests with a manual timeout, but since the underlying `URLSession` task is never cancelled, each timed-out request keeps running and holds one of `URLSession.shared`'s limited per-host connections until its own, much longer, timeout fires. These orphaned requests pile up faster than they clear, exhaust the connection pool, and new requests queue behind them. This PR now stores the URLSession task alongside the WKURLSchemeTask (as an associated object in an extension, like `stopped`), so that when `stop()` is called, we can cancel the request. There's a test as well, that fails prior to this patch and passes with it. It just shows that, previously, calling stop() from the web view did not in fact stop the underlying network request, while now it does. Note that this change only affects iOS; I am unclear if the Android implementation has similar issues, but that's out of scope for this PR. Additionally calling out that adding an internal `urlSession` property instead of hardcoding `URLSession.shared` is a change that technically only exists for testing, but as an internal-only change I don't think it's egregious here. If you wanted to merge this change without merging the test, I'd remove that as well.
c1e6786 to
3e5ff17
Compare
|
@coderabbitai review |
|
3 participants
Merge Conflict Review Required
The sync of upstream PR ionic-team#8546 from @lazerwalker encountered merge conflicts.
Original PR: ionic-team#8546
What happened
Synced from upstream by Capacitor+ Bot
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.