-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Migrate Connectivity Monitoring to NWPathMonitor on Apple Platforms #16112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
cherylEnkidu
wants to merge
33
commits into
main
Choose a base branch
from
cheryl/SCNetworkReachability
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
295ee89
refactor ConnectivityMonitorApple to use NW_PATH_MONITOR
cherylEnkidu 6e95d71
format
cherylEnkidu 6a82fbd
add Network package dependency
cherylEnkidu 2d6a1cb
add test
cherylEnkidu 11a630d
add include file
cherylEnkidu 20e84db
format
cherylEnkidu 4309cfb
add test availability
cherylEnkidu 4ab254d
add dummy callback
cherylEnkidu 2f95e78
add test log
cherylEnkidu 191630c
add debuglog
cherylEnkidu a5fab0d
format
cherylEnkidu 25d5431
format
cherylEnkidu 3bd3498
add shutdown protector
cherylEnkidu 0ca806a
add shutdown protection to auth
cherylEnkidu dea868d
move the activity monitor to the main thread
cherylEnkidu d8b7386
add debug log
cherylEnkidu 52fb772
upload xcode log
cherylEnkidu b9bfb5d
revert the changes in app check and auth to isolate the problem
cherylEnkidu 8d579ac
revert the changes in grpc to isolate issue
cherylEnkidu 5e815cd
add safeguard to connectivity monitor
cherylEnkidu 434d435
Improve the code
cherylEnkidu 3495380
require test to explicitly destroy
cherylEnkidu 85cd6d1
Merge branch 'main' into cheryl/SCNetworkReachability
cherylEnkidu f7b4d53
remove debug print
cherylEnkidu d37a9f3
remove unwanted changes
cherylEnkidu 5927991
add more tests
cherylEnkidu 2253f47
add changelog
cherylEnkidu 0862789
fix bug and format code
cherylEnkidu 55dad88
add cmake network library dependency
cherylEnkidu 5b44b78
make sure the internet connection failure would not lead to crash
cherylEnkidu 811c9b6
add comments
cherylEnkidu 419b433
improve format style
cherylEnkidu 4493f86
remove the function which drain the monitor queue in destructor
cherylEnkidu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix flaky testWatchSurvivesNetworkDisconnect under new connectivity timing.
The test used
addSnapshotListenerWithIncludeMetadataChanges:YESand a filter of!empty && !fromCache, expecting the listener to fulfill the expectation exactly once. With metadata changes enabled, the listener fires twice in the recovery sequence: once when the snapshot is raised from the server with hasPendingWrites=true (write enqueued, awaiting ack), and again when hasPendingWrites flips to false after server ack. Both pass the existing filter, causing over-fulfill.This is a latent bug in the test, not a regression from the connectivity monitor refactor. The connectivity changes in this PR shifted the timing of network state delivery in a way that exposed it.
Tighten the filter to also require !hasPendingWrites, which is the condition the test actually intends to wait for: the write has fully round-tripped to the server.