Status Update
Comments
ub...@gmail.com <ub...@gmail.com> #2
Now, instead of crashing, we ignore invalid deep links, printing a log message saying as much.
ub...@gmail.com <ub...@gmail.com> #3
The problem still occurs on version 2.2.0-rc04
ub...@gmail.com <ub...@gmail.com> #4
da...@google.com <da...@google.com> #5
ub...@gmail.com <ub...@gmail.com> #6
ub...@gmail.com <ub...@gmail.com> #7
da...@google.com <da...@google.com> #8
NavDestination node = i == 0 ? mGraph : graph.findNode(destinationId);
if deepLink.length == 1 node never will be null and this will cause IllegalStateException
Bug demo:
get NavigationAdvancedSample from github
on R.id.about_btn click
NavDeepLinkBuilder(it.context)
.setGraph(R.navigation.list)
.setDestination(R.id.leaderboard)
.createTaskStackBuilder()
.startActivities()
will crash because R.id.leaderboard is start destination (findInvalidDestinationDisplayNameInDeepLink will have deepLink length == 1)
NavDeepLinkBuilder(it.context)
.setGraph(R.navigation.list)
.setDestination(R.id.userProfile)
.createTaskStackBuilder()
.startActivities()
will work because findInvalidDestinationDisplayNameInDeepLink will return NOT null
ub...@gmail.com <ub...@gmail.com> #9
It still crash with the latest version 2.2.1. I think the problem lies somewhere around this code
NavDestination node = i == 0 ? mGraph : graph.findNode(destinationId);
When i
is 0
, just return mGraph
itself. When the deep link destination is a root of bottom_nav tab, the deeplink.length
becomes 1. Which means this function still says there's no invalid Destination in deep link even if deepLink[0]
is totally irrelevant tomGraph
.
So this step passes and when findDestination() is actually triggered down the road, it won't find any destination and thus lead to unknown destination
crash.
All NavControllers of bottom nav automatically call handleDeepLink()
as part of onGraphCreated()
. The problem arises the moment NavController whose graph is irrelevant to the root deep link destination calls handleDeepLink()
.
yb...@google.com <yb...@google.com> #10
It still crashes with 2.3.0-alpha03 version but only if you try to deeplink to startDestination fragment inside navigation graph. Repro steps (described in #8, but lets repeat it in hope issue will be reopened):
- download NavigationAdvancedSample
- send pending intent to any startDestination screen, e.g.
val pendingIntent = NavDeepLinkBuilder(applicationContext)
.setGraph(R.navigation.list)
.setDestination(R.id.leaderboard)
.setArguments(bundle) // no matter if it is empty in this use case
.createPendingIntent()
pendingIntent.send()
Exception:
Caused by: java.lang.IllegalStateException: unknown destination during deep link: com.example.android.navigationadvancedsample:id/list
at androidx.navigation.NavController.handleDeepLink(NavController.java:689)
at androidx.navigation.NavController.onGraphCreated(NavController.java:602)
at androidx.navigation.NavController.setGraph(NavController.java:563)
at androidx.navigation.NavController.setGraph(NavController.java:528)
at androidx.navigation.fragment.NavHostFragment.onCreate(NavHostFragment.java:246)
at androidx.fragment.app.Fragment.performCreate(Fragment.java:2684)
at androidx.fragment.app.FragmentStateManager.create(FragmentStateManager.java:280)
at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1175)
at androidx.fragment.app.FragmentTransition.addToFirstInLastOut(FragmentTransition.java:1255)
at androidx.fragment.app.FragmentTransition.calculateFragments(FragmentTransition.java:1138)
at androidx.fragment.app.FragmentTransition.startTransitions(FragmentTransition.java:136)
at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1989)
at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1947)
at androidx.fragment.app.FragmentManager.execSingleAction(FragmentManager.java:1818)
at androidx.fragment.app.BackStackRecord.commitNow(BackStackRecord.java:297)
at com.example.android.navigationadvancedsample.NavigationExtensionsKt.obtainNavHostFragment(NavigationExtensions.kt:269)
at com.example.android.navigationadvancedsample.NavigationExtensionsKt.setupWithNavController(NavigationExtensions.kt:61)
at com.example.android.navigationadvancedsample.MainActivity.setupBottomNavigationBar(MainActivity.kt:121)
at com.example.android.navigationadvancedsample.MainActivity.onCreate(MainActivity.kt:54)
Reopening would be a good sign that issue is recognized.
ub...@gmail.com <ub...@gmail.com> #11
Just in a case Reopen feature is not implemented in IssueTracker :), new ticket is created:
yb...@google.com <yb...@google.com> #12
ub...@gmail.com <ub...@gmail.com> #13
da...@google.com <da...@google.com> #14
Filing a new bug with a sample project that reproduces the issue you're still seeing is indeed the correct behavior. We don't support bug necromancy, particularly 30 releases after the original fix was shipped.
yb...@google.com <yb...@google.com> #15
#13 i'm curious what went wrong w/ the Github setup. I don't want to hijack this bug but if you can either file an issue at
el...@google.com <el...@google.com>
ub...@gmail.com <ub...@gmail.com> #16
I don't think I will add the test; ultimately I am unfamiliar with AOSP tooling and in particular the test setup for this, so it's an open-ended time commitment, and I already spent too much time on this tiny code problem.
I can still make a PR for the code change that fixes the bug (based on my testing via repro & actual app), but it's so trivial that you might just as well change it yourselves. As I mentioned in #10, the primary concern with the code change would be regressions, which are not captured by the envisioned test anyway, so I'd consider it defensible to go without test. The alternative, no fix, leaves TRUNCATE in a poor state.
ub...@gmail.com <ub...@gmail.com> #17
I meant
yb...@google.com <yb...@google.com> #18
are you still blocked on the SSH issue? FYI we do have other people sending PRs from Github without that issue so I would like to figure out what is broken in your case (as it might be affecting other people as well).
I want to respect your time though so if you don't want to deal with it anymore, you can send a PR without tests and we'll take it from there to add tests (though unfortunately, we cannot merge a PR without tests so we'll need to add them).
ub...@gmail.com <ub...@gmail.com> #19
Thanks for your and Aurimas' help on the SSH issue last year, doing a checkout with filter and SSH caused the problems. I suggested updating the online instructions to point out this gotcha, not sure what happened after that on your end.
I am sending you a PR with the fix & added test. I gave it one more push to get if off my mind.
el...@google.com <el...@google.com>
ap...@google.com <ap...@google.com> #20
Branch: androidx-main
commit a5c2900ca26e8f24d4492e5a0e90f0669702c680
Author: Uli Bubenheimer <bubenheimer@users.noreply.github.com>
Date: Mon Apr 19 09:22:25 2021
[GH] Fix intermittent InvalidationTracker issues in JournalMode.TRUNCATE
## Proposed Changes
- Fixes issue in Room JournalMode.TRUNCATE where the InvalidationTracker callback is sometimes invoked invalidly, too late, or not at all.
- Adds InvalidationTrackerBehavioralTest
## Testing
Test: ./gradlew test connectedCheck -x :room:room-benchmark:cC -x :room:integration-tests:room-incremental-annotation-processing:test
## Issues Fixed
Fixes:
This is an imported pull request from
Resolves #159
Github-Pr-Head-Sha: b32fc85dd1368db5fe046b1edfecdb82ac3e5478
GitOrigin-RevId: 169da50d9ce5d0be6aa686d603d51a380f46ab63
Change-Id: I0c04f25303043f45c8efe687df93f7122acbc7d4
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/InvalidationTrackerBehavioralTest.java
M room/runtime/src/main/java/androidx/room/InvalidationTracker.java
Description
Component used: Room InvalidationTracker
Version used: 2.2.5
Devices/Android versions reproduced on: Android Studio emulator with Android R DP 2.1
Sample project:https://github.com/bubenheimer/invalidationtrackerbug
I am seeing consistency problems with the updates from InvalidationTracker when using JournalMode.TRUNCATE.
In my production app some updates are skipped - the observer is never called for a percentage of updates to a rarely updated table, causing the associated app screen to not get updated to the current state. I debugged into it and believe that this is associated with a lack of transactionality in InvalidationTracker.mRefreshRunnable. JournalMode.WRITE_AHEAD_LOGGING uses a transaction here, while JournalMode.TRUNCATE does not. Reads and writes of the table update log are disassociated, leaving the door open to overwriting recent changes to the log.
In the provided sample project I see the opposite effect, however: a lot of extra updates are triggered by InvalidationTracker with JournalMode.TRUNCATE. I am not sure what the underlying mechanism is for this behavior.
Exact effects are timing-related. The bottom line is that InvalidationTracker looks essentially broken for JournalMode.TRUNCATE. On the other hand I do not see significant issues with JournalMode.WRITE_AHEAD_LOGGING.