feat(native_dio_adapter): opt-in Cronet provider fallback#2573
Merged
Conversation
Add `NativeAdapter(createFallbackAdapter: ...)` for Android devices on which every installed Cronet provider is disabled (for example AOSP emulators or devices without Google Play services). When Cronet reports the specific provider-disabled `RuntimeException`, the caller-supplied factory returns any `HttpClientAdapter` and requests continue via that adapter. The option is opt-in and defaults to `null`: without it, `NativeAdapter` continues to use Cronet and propagates initialization errors exactly as before. Detection matches the exact provider-disabled message on a `JniException`; every other Cronet error is propagated unchanged, and adapter selection is sticky for the lifetime of the `NativeAdapter`. Fixes #2444. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
4 tasks
CaiJingLong
approved these changes
Jul 22, 2026
CaiJingLong
left a comment
Contributor
There was a problem hiding this comment.
Review Conclusion: Approve
Change Summary
Adds an opt-in createFallbackAdapter factory parameter to NativeAdapter. On Android, when CronetEngine.build() throws the specific provider-disabled RuntimeException (all Cronet providers disabled, e.g. AOSP emulators / devices without Google Play services), the caller-supplied factory returns any HttpClientAdapter and requests continue via that adapter. The parameter defaults to null; without it, behavior is unchanged. Fixes #2444.
Review Details
| Dimension | Conclusion | Notes |
|---|---|---|
| Correctness | Pass | Lazy selection in _selectAdapter() is synchronous with no await points, so no race in a single Dart isolate. The try/catch wraps only _buildCronetAdapter(), so post-init errors (connection/TLS/timeout/redirect/cancellation) from a successfully-built Cronet adapter propagate unchanged — verified by test. The classifier isCronetProviderUnavailable requires error is JniException and message.contains(cronetProvidersDisabledMessage) with the exact java.lang.RuntimeException: prefix, so a wrong exception class or a non-JniException carrying the same text is rejected. Sticky selection via _selected is correct. close() is idempotent (_closed flag) and does not build Cronet when no request was made. |
| Tests | Pass | 12 new focused tests in cronet_fallback_adapter_test.dart using a @visibleForTesting build seam and recording mock adapters. Covers: classifier positive/negative (wrong class, wrong message, non-JniException), fallback invoked exactly once with identity-preserved request/body/cancel, non-matching rethrow, ArgumentError rethrow, post-init errors do not trigger fallback, sticky reuse of both fallback and Cronet paths, close after selection (exactly once, force forwarded), close before any request (no build, no fallback). Full package suite: 26 tests pass. dart analyze clean. |
| Style | Pass | Matches existing module boundaries and naming. CreateFallbackAdapter typedef exported via show only — CronetWithFallbackAdapter, the classifier, and internals are not part of the public API. No debug remnants. dart format clean. |
| Risk | Pass | Additive, opt-in, null default — no behavior change for existing apps, no public-API break. Direct jni: ^0.14.0 dependency is justified (needed for the typed JniException match) and compatible with cronet_http: ^1.5.0's transitive jni: ^0.14.2 (resolves to 0.14.2). The eager CronetEngine.build() in the fallback path vs. the lazy build in the non-fallback path is intentional — it is the mechanism that surfaces the provider-disabled error before any request is delegated. Sensitive area (network adapter), but the change is well-isolated behind the opt-in parameter. |
| Docs | Pass | README.md adds an opt-in section with the observable-behavior caveat. CHANGELOG.md Unreleased entry is user-facing and references #2444. API doc on the new parameter is thorough (contract, stickiness, close semantics, platform scope, example). |
| Repo constraints | Pass | Branch feat/cronet-provider-fallback follows category/short-description. Commit uses Conventional Commits (feat(native_dio_adapter): ...), lowercase subject, no gitmoji+prefix mix. Co-Authored-By AI attribution present. CHANGELOG updated. No public-API break. No drive-by dependency bumps — the jni addition is the point and is documented. |
Confirmed Key Points
- Correctness: The classifier is precise (typed
JniException+ exactRuntimeExceptionmessage viacontainsto tolerate the appended Java stack trace). Post-init errors are untouched because the try/catch scope is only the build seam. Selection is sticky;close()is idempotent and never initializes Cronet when no request was made. - Tests: 12 new tests + 14 existing = 26 pass;
dart analyzeanddart formatclean. Coverage on the new file is 78.26%. - Risk: Opt-in with
nulldefault — zero behavior change for non-opting apps. Thejnidirect dependency is compatible with the existing transitive resolution. No public symbols removed/renamed; only an additive optional parameter and a new exported typedef.
Suggestions (non-blocking)
- The PR body references
docs/plans/cronet-provider-fallback.md, but that file is not part of the diff. Consider either including it or dropping the reference to avoid a dangling citation. - Android integration on an AOSP emulator without Google Play services remains unverified (acknowledged in the PR body). The unit tests use a controllable build seam and mock the
JniException; real-device validation would close the loop on the exact runtimeJniException.messageshape. - Pre-existing (not introduced by this PR): the package declares
sdk: '>=3.1.0'whilecronet_http: ^1.5.0transitively pullsjni: ^0.14.2which requiressdk: '>=3.3.0'. This constraint mismatch predates this PR and is not worsened by the added directjni: ^0.14.0dependency.
This comment was generated by AI agent omp (model: devin/glm-5-2).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2444.
Adds an opt-in
createFallbackAdapterfactory toNativeAdapter. When Cronet reports that every installed provider on the device is disabled (for example, AOSP emulators or devices without Google Play services), the caller-supplied factory returns anyHttpClientAdapterand requests continue via that adapter — instead of every request failing onCronetEngine.build().Key properties
null. Without it,NativeAdaptercontinues to use Cronet and propagates initialization errors exactly as it does today. No behavior change for existing applications.RuntimeExceptionon a typedJniException(addsjnias a direct dependency). Does not fall back on any otherJniException, non-JniExceptionerrors carrying similar text, or engine initialization errors of other kinds.fetchand cached; later requests do not probe Cronet again.close()closes the selected adapter once. Closing before the first request does not initialize Cronet or create the fallback.Client. The factory returns anyHttpClientAdapter— callers pick the one whose TLS, proxy, cookie, transport, and observability behavior matches their needs.Why this shape
This PR replaces the two prior attempts against #2444, which were rejected on their approach:
IOClientwhenCronetEngineinitialization fails #2445 wrappedCronetEngine.build()inCronetAdapterand switched the internalhttp.Clientmid-request. That is not safe onceConversionLayerAdapterhas consumed the Dio request stream.HttpClientAdapter()(dart:io), and matched on.toString(). That silently changes networking behavior for existing apps and is fragile against exception-message changes.The rationale for the current design is captured in
docs/plans/cronet-provider-fallback.md.Changes
plugins/native_dio_adapter/lib/src/cronet_fallback_adapter.dart— new Android-only lazy selection wrapperCronetWithFallbackAdapter, the classifierisCronetProviderUnavailable, and theCreateFallbackAdaptertypedef. Includes a@visibleForTestingconstructor that lets unit tests inject a controllable "build cronet path" seam without native code.plugins/native_dio_adapter/lib/src/native_adapter.dart— newcreateFallbackAdapterparameter; wired on Android only, only when the factory is supplied.plugins/native_dio_adapter/lib/native_dio_adapter.dart— export theCreateFallbackAdaptertypedef.plugins/native_dio_adapter/pubspec.yaml— add directjni: ^0.14.0dependency (compatible withcronet_http: ^1.5.0's transitive^0.14.2).plugins/native_dio_adapter/README.md— opt-in example section with the observable-behavior caveat.plugins/native_dio_adapter/CHANGELOG.md— Unreleased entry.plugins/native_dio_adapter/test/cronet_fallback_adapter_test.dart— 12 focused tests using a controllable "build cronet adapter" seam and mockHttpClientAdapterimplementations.Test plan
melos run format— passes.melos run analyze— passes.melos run test— passes (26 tests innative_dio_adapter, including 12 new).Unit coverage (per plan)
JniException(wrong exception class or wrong message) is rethrown and no fallback is created.JniExceptionerror carrying the same text is rejected by the classifier.ArgumentErrorduring Cronet build is rethrown, not fallen back.closeafter selection closes the selected adapter exactly once (and laterclosecalls are no-ops).closebefore any request does not invoke the build seam or create a fallback.The "no fallback factory" case is satisfied by construction —
NativeAdapterdoes not instantiate the wrapper when the factory isnull, and calling with anullfactory is indistinguishable from prior versions.🤖 Generated with Claude Code