feat(cupertino_http): shared session#1876
feat(cupertino_http): shared session#1876mertalev wants to merge 21 commits intodart-lang:masterfrom
Conversation
github-actions
bot
added
package:cupertino_http
PR Health
Coverage
|
| File | Coverage |
|---|---|
| pkgs/cupertino_http/example/integration_test/client_conformance_test.dart | 💔 Not covered |
| pkgs/cupertino_http/example/integration_test/client_test.dart | 💔 Not covered |
| pkgs/cupertino_http/example/integration_test/url_session_delegate_test.dart | 💔 Not covered |
| pkgs/cupertino_http/example/integration_test/url_session_test.dart | 💔 Not covered |
| pkgs/cupertino_http/example/integration_test/web_socket_conformance_test.dart | 💔 Not covered |
| pkgs/cupertino_http/lib/src/cupertino_api.dart | 💔 Not covered |
| pkgs/cupertino_http/lib/src/cupertino_client.dart | 💔 Not covered |
| pkgs/cupertino_http/lib/src/cupertino_web_socket.dart | 💔 Not covered |
| pkgs/cupertino_http/lib/src/native_cupertino_bindings.dart | 💔 Not covered |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
Breaking changes ✔️
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| cupertino_http | Breaking | 2.4.0 | 3.0.0-wip | 3.0.0-wip | ✔️ |
This check can be disabled by tagging the PR with skip-breaking-check.
Unused Dependencies ⚠️
| Package | Status |
|---|---|
| cupertino_http | ❗ Show IssuesThese packages are used outside lib/ but are not dev_dependencies: |
For details on how to fix these, see dependency_validator.
This check can be disabled by tagging the PR with skip-unused-dependencies-check.
Changelog Entry ✔️
| Package | Changed Files |
|---|
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check.
License Headers ✔️
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
| Files |
|---|
| no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/http/example/main.dart |
| pkgs/http_multi_server/test/cert.dart |
This check can be disabled by tagging the PR with skip-license-check.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
| Package | Leaked API symbol | Leaking sources |
|---|
This check can be disabled by tagging the PR with skip-leaking-check.
|
I just looked into per-task delegates and I think they'd also work for this. Any hooks the task delegate doesn't override go to the session-level delegate, so things like client certificates would still work. It'd avoid the limitations with websockets and redirect tracking, so maybe that's the way to go. I'll give it a try later. |
|
Hi @mertalev this is very high quality but I'll need a while to review it.
If I understand correctly, the current approach can't be made to honor max-redirects, right? But will the redirect handler attached to the session be called? Also, I guess that you are doing something like: final sharedSession = URLSession.sessionWithConfiguration(
..., onFinishedDownloading: onFinished);
void foo() {
sharedSession.downloadTaskWithRequest(...);
}
void bar() { // Possibly in another isolate
final client = CupertinoClient.fromSharedSession(sharedSession);
client.
}The @liamappelbe is there a way to generate Dart bindings for Swift-only methods like these: https://developer.apple.com/documentation/foundation/urlsession/bytes(from:delegate:)?language=objc |
In theory you could run this through swiftgen, and it would take care of generating an |
Yup, the Dart delegate vs Dart delegate vs task-level delegate: As for usage, I create a session in
Probably yes! |
|
I moved the legacy path to use Dart with |
|
If we were willing to limit support to iOS 15+ and macOS 12.0+, could we accomplish the shared session using a per-task delegate implemented in Dart? I was experimenting with that approach and got stuck when I hit this issue: dart-lang/native#3119 |
|
The Dart binding approach simplified the code quite a bit! Swift ended up just being extra indirection. |
3 participants
This PR enables sharing of a single URLSession with platform and across isolates. The existing implementation's delegate approach tightly couples a session to a single isolate. As a solution, it adds an implementation based on the
bytes(for:)API, which does not require delegate-based callbacks. This allows users to bring their own session configured with their own delegate. It enables my goal to use a single shared session in my app; this is optimal for multiplexing and caching and means a single place to configure authentication like client certificates.I've tested these changes in the app and made sure the conformance tests are passing with this implementation. A few changes were needed in the tests because
bytes(for:)does its own chunking internally before yielding bytes. I have not tested its performance. I'd guess it performs better since the implementation is more contained in Swift with less Dart trampolining/bookkeeping, but it'd be good to profile it.From a maintenance standpoint, I think this implementation is simpler than the delegate-based approach and might be able to replace it at some point, perhaps when the minimum iOS version is bumped to 15? It's relatively self-contained as it is, though, and should have no effect on existing users of cupertino_http.
Fixes #1002
Contribution guidelines:
dart format.Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
Note: The Dart team is trialing Gemini Code Assist. Don't take its comments as final Dart team feedback. Use the suggestions if they're helpful; otherwise, wait for a human reviewer.