Skip to content
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

IllegalArgumentException: Site successfully fetched but has not been found in the local db. #11394

Closed
sentry-io bot opened this issue Feb 27, 2020 · 7 comments · Fixed by #11446
Closed

Comments

@sentry-io
Copy link

sentry-io bot commented Feb 27, 2020

Sentry Issue: WORDPRESS-ANDROID-27A

IllegalArgumentException: Site successfully fetched but has not been found in the local db.
    at org.wordpress.android.ui.sitecreation.previews.SitePreviewViewModel$fetchNewlyCreatedSiteModel$1.invokeSuspend(SitePreviewViewModel.kt:207)
...
(2 additional frame(s) were not displayed)

Site successfully fetched but has not been found in the local db.
@malinajirka malinajirka added this to To Do in Groundskeeping via automation Feb 27, 2020
@malinajirka malinajirka moved this from To Do to Prioritized Android in Groundskeeping Feb 27, 2020
@designsimply
Copy link
Contributor

Events in the last 90 days: 3,100
Users affected in the last 90 days: 2,300
First seen in: WPAndroid 12.6
https://sentry.io/share/issue/df015c467a0e4b08895e4cf1875484c9/

@develric develric self-assigned this Mar 9, 2020
@develric
Copy link
Contributor

develric commented Mar 10, 2020

Investigated this one a bit, sharing what I learned so far hoping it can help in some way. As a side note, I spent some time to try to figure out how to filter events in sentry based on breadcrumbs content but didn't find a way so some of the notes below are based on sampling the events data.

  1. Able to reproduce: No
  2. Limited to specific Android version: No
  3. Limited to specific App version: No. But this said there are more cases in percentage with version >= 14.0 that was released end of Jan. Here below a graph showing this trend.

image

  1. The issue seems an effect of this event w/WordPress-DB: Can't insert WP.com site XYZ, missing user account that happens in all the events I sampled and is the result of this fluxC check in SiteSqlUtils.java below where
    we look for Accounts with USER_ID <> 0 and we cannot find.
public static int insertOrUpdateSite(SiteModel site) throws DuplicateSiteException {
...
        if (site.isUsingWpComRestApi()) {
            List<AccountModel> accountModel = WellSql.select(AccountModel.class)
                    .where()
                    .not().equals(AccountModelTable.USER_ID, 0)
                    .endWhere()
                    .getAsModel();
            if (accountModel.isEmpty()) {
                AppLog.w(T.DB, "Can't insert WP.com site " + site.getUrl() + ", missing user account");
                return 0;
            }
        }
...
  1. This one is based on sampling the last 2 months events but it seems the flow is: SIGNUP with Google -> Create a new site (doing it as last part of the signup flow). This can be just because users use most the signup with google flow or also that the sampling of the events I did is biased of course. For record I was able using the current released app version 14.2 to complete the above flow without issues.

Related to the item 4 above I can think to 2 things:

  • For some reason we fail to insert the account record into the AccountModel table, so we actually do not have the user account when we need it (I actually could not find a related error in the events I looked into)
  • The account record is there but it has a USER_ID = 0

Maybe could help to have the above answered inserting some additional logging (both in the function above and/or in the code section where we get the user_id from API to check if eventually we got a 0 user_id from that)

I'm currently asking access to Google Console to be enabled to debug the signup with google flow so was not able yet to debug it specifically and could not find reading the code something that can make me think to a possible related issue.

Pinging @jd-alexander , @khaykov , @renanferrari (I think you are in GK as well, in case not sorry for this 😜). If one of you has some time to go into this one a bit let me know if you agree/disagree with the above analysis and adding the logs or have some better clue on the cause.

I will monitor if I get my account fully enabled for the google console so to make some more debug on the above.

@develric develric removed their assignment Mar 10, 2020
@renanferrari
Copy link
Member

Thanks for the thorough investigation @develric, much appreciated. I'm going to take a look at this between today and tomorrow and I'll let you know if I'm able to contribute in any way.

@renanferrari renanferrari self-assigned this Mar 11, 2020
@jd-alexander
Copy link
Contributor

Thanks for looking into this @develric and great for picking up @renanferrari Looking forward to hearing your findings!

@develric
Copy link
Contributor

Hi there 😊, just wanted to share I finally was able to get the SignUp with Google flow up and go into some debugging but unfortunately could not reproduce the issue.

@renanferrari
Copy link
Member

Just a quick update to say that I was able to reproduce this error earlier today. Will update this issue tomorrow with the steps to reproduce it and a possible fix.

@renanferrari
Copy link
Member

Steps to reproduce this issue

  1. Use a Google account to create a new WordPress.com account and make sure you don't create any sites with it.
  2. Clear app data.
  3. On the initial screen, tap Sign up for WordPress.com and then Sign up with Google.
  4. Notice the Google account picker dialog.
  5. Pick a Google account that already has a WordPress.com account associated with it.
  6. Notice the Post-Signup Interstitial screen.
  7. Tap Create WordPress.com Site.
  8. Skip the next two steps.
  9. Choose a domain name and tap Create Site.
  10. Depending on the Android version, you might either notice:
    • An error message for the crash and have the app be completely closed (demonstrated below).
    • No errors and just be abruptly redirected to the main screen of the app.
      • Note: in both cases the site is successfully created in the background. However, in this case, it may look like the site wasn't created at all, because it won't appear anywhere until the app is restarted - which might be even more confusing than seeing the app crashing.

crash

Findings

TL;DR: Sometimes we must show the Login Epilogue Screen instead of the Post Signup Interstitial Screen at the end of the login flow, so that we can retrieve some user information needed for the app to properly work after a login. We assumed this wasn't needed for Google signups/logins, but it turns out we do have to do that on the very specific scenario of an user trying to sign up with an existing Google account.

As soon as @develric mentioned that this issue happened at the end of the sign up flow and had seen a spike since the release of version 14.0, I realized it might be related to the introduction of the Post Signup Screen (Master issue: #10918 | Final PR: #11024).

This screen would be shown once, after the completion of the signup or login flow, if the user account had no sites. Initially, that's exactly what we did, but then we realized that showing the last screen of this flow (the Login/Signup Epilogue Screen) to an user with no sites didn't provide much value, since it would basically be an empty screen in those cases (example).

Because of that, we decided to skip those epilogue screens and replace them with the new interstitial screen for users that had no sites (you can see the result here), which worked pretty well for the sign up flow, but we soon found some problems that made us go back on that approach for Magic Link logins (#11020).

While trying to work on a fix for that, we found that although the Login Epilogue Screen didn't provide much value to an user with no sites, its current implementation performs an important flow control responsible for triggering some actions needed for the app to work correctly after some login cases (Magic Link included). More specifically, when the LoginEpilogueFragment receives an argument doLoginUpdate of true it becomes responsible to fetch the user account and settings (including sites) itself.

Now, I never really understood why that was needed for some cases and not for others. My best guess at the time was that this was somehow related to the WPMainActivity handling some asynchronous callbacks needed for some login cases. As you may realize when you look into that part of the code, the login/signup flow often relies on implicit behaviors (that could even be characterized as collateral effects), which makes it harder to debug and fully comprehend what's the expected behavior.

That being said, what we missed at the time was that this was also needed for this very specific case. Here's a recap of all the pre-conditions that have to take place for this issue to appear:

  1. The user must have already created a WordPress.com account with a Google account.
  2. The user must not have created any sites with that account.
  3. The user must be using a new device (or be logged out of a device that has never been used to create new accounts or sites after version 14.0).
  4. The user must try to sign up with that existing Google account instead of logging in.
  5. And finally, the user must complete the site creation flow from the Post Signup Interstitial screen.

The good news is that the fix is really simple: we just need to make sure we are not skipping the Login Epilogue Screen for those cases. We will probably also have the opportunity to revisit these flows pretty soon and work on how we can make them more testable and debuggable.

Here's the link for the PR I opened with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Groundskeeping
  
Done Mar 9-13, 2020
5 participants