Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

1084: Update app services to version 0.46.0 #1124

Merged
merged 4 commits into from
Jan 16, 2020

Conversation

eliserichards
Copy link
Contributor

Fixes #1084

Testing and Review Notes

Screenshots or Videos

To Do

  • add “WIP” to the PR title if pushing up but not complete nor ready for review
  • double check the original issue to confirm it is fully satisfied
  • add testing notes and screenshots in PR description to help guide reviewers
  • make sure CI builds are passing (e.g.: fix lint and other errors)

@eliserichards eliserichards requested a review from a team as a code owner January 3, 2020 23:17
jhugman
jhugman previously approved these changes Jan 6, 2020
Copy link
Contributor

@jhugman jhugman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems painless!

@eliserichards
Copy link
Contributor Author

Not quite that easy! The state is stuck in a login loop:

Logcat from first try logging in:

2020-01-06 16:56:38.418 15158-15548/mozilla.lockbox D/fxaclient_ffi: fxa_to_json
2020-01-06 16:56:38.460 15158-15547/mozilla.lockbox I/FirefoxAccount: Executing: getProfile
2020-01-06 16:56:38.462 15158-15547/mozilla.lockbox D/fxaclient_ffi: fxa_profile
2020-01-06 16:56:38.470 15158-15321/mozilla.lockbox I/FirefoxAccount: Executing: get access token
2020-01-06 16:56:38.957 15158-15547/mozilla.lockbox D/fxaclient_ffi: fxa_to_json
2020-01-06 16:56:38.957 15158-15547/mozilla.lockbox W/WrappingPersistenceCallback: InternalFxAcct tried persist state, but persistence callback is not set
2020-01-06 16:56:38.959 15158-15321/mozilla.lockbox D/fxaclient_ffi: fxa_get_access_token
2020-01-06 16:56:38.969 15158-15547/mozilla.lockbox I/FirefoxAccount: Successfully executed: getProfile
2020-01-06 16:56:39.065 15158-15321/mozilla.lockbox D/fxaclient_ffi: fxa_to_json
2020-01-06 16:56:39.066 15158-15321/mozilla.lockbox W/WrappingPersistenceCallback: InternalFxAcct tried persist state, but persistence callback is not set
2020-01-06 16:56:39.077 15158-15321/mozilla.lockbox I/FirefoxAccount: Successfully executed: get access token
2020-01-06 16:56:40.086 15158-15619/mozilla.lockbox D/fxaclient_ffi: fxa_get_token_server_endpoint_url

on second try:

2020-01-06 16:59:15.901 15158-15603/mozilla.lockbox W/WrappingPersistenceCallback: InternalFxAcct tried persist state, but persistence callback is not set
2020-01-06 16:59:15.902 15158-15603/mozilla.lockbox I/FirefoxAccount: Successfully executed: getProfile
2020-01-06 16:59:15.903 15158-15674/mozilla.lockbox D/fxaclient_ffi: fxa_get_token_server_endpoint_url
2020-01-06 16:59:15.905 15158-15608/mozilla.lockbox D/logins_ffi: sync15_passwords_state_new
2020-01-06 16:59:16.003 15158-15608/mozilla.lockbox E/logins::ffi: Not a database / invalid key error```

@jhugman
Copy link
Contributor

jhugman commented Jan 10, 2020

I have been to get to the item list screen:

  • I have signed in multiple times from first install.
  • I've restarted the app, once signed in.
  • I've logged out, and then logged in with a different account.

I've been unable to get to the item list screen:

  • I've locked the app, then killed it, then started again. I have done this quite a few times, so haven't found a reliable STR.
D glean/MetricsPingSched: The 'metrics' ping was last sent on Fri Jan 10 00:00:00 GMT 2020
I glean/MetricsPingSched: The 'metrics' ping was already sent today, Fri Jan 10 12:44:36 GMT 2020.
D glean/MetricsPingSched: Scheduling the 'metrics' ping in 54923474ms
D RustNativeSupport: findMegazordLibraryName(logins, 0.42.0
D RustNativeSupport: lib in use: none
D RustNativeSupport: lib configured: megazord
D RustNativeSupport: lib version configured: 0.42.0
D RustNativeSupport: settled on megazord
E logins::ffi: Not a database / invalid key error
E ResolverController: No valid NAT64 prefix (198, <unspecified>/0)
D fxaclient_ffi: fxa_to_json
W WrappingPersistenceCallback: InternalFxAcct tried persist state, but persistence callback is not set
D fxaclient_ffi: fxa_get_token_server_endpoint_url
I FirefoxAccount: Successfully executed: getProfile
D logins_ffi: sync15_passwords_state_new
E logins::ffi: Not a database / invalid key error

lib version configured: 0.42.0 looks super suspicious.

@jhugman
Copy link
Contributor

jhugman commented Jan 10, 2020

Regarding the lockbox-android/build.gradle file, we see:

    // Determined from
    // https://github.com/mozilla-mobile/android-components/blob/v19.0.1/buildSrc/src/main/java/Dependencies.kt,
    // where the version in the URL is the tag corresponding to `android_components_version`.
    ext.mozilla_appservices_version = '0.46.0'

So it looks like we have a mismatch between Lockwise's version of logins and android-components version.

I suspect the least painful thing to do here is to bump the A-C version up to a version such that A-S is high enough, then get our dependency versions (i.e. A-S and Glean) to match those versions.

We should also review if these dependencies are being included twice.

@eliserichards eliserichards force-pushed the 1084-update-appservices branch from 9284011 to 258d096 Compare January 13, 2020 16:30
@eliserichards
Copy link
Contributor Author

The latest version of android components - 27.0.4 - references version 0.44.0 of app services. Looks like everything is working as expected once I updated that. Good catch.

@eliserichards eliserichards force-pushed the 1084-update-appservices branch from 258d096 to e1c3b98 Compare January 13, 2020 16:32
@eliserichards eliserichards changed the title [WIP] 1084: Update app services to version 0.46.0 1084: Update app services to version 0.46.0 Jan 13, 2020
@jhugman
Copy link
Contributor

jhugman commented Jan 13, 2020

Is Glean affected by this too? We should check that the glean version is the same as the one shipping with A-S and A-C.

@eliserichards eliserichards requested a review from jhugman January 13, 2020 21:58
@eliserichards
Copy link
Contributor Author

Tried to look for any places we may have had null usernames/pwds instead of "" empty string. Only found one in a test. Let me know if you can think of other places that might break with the null -> "" change @jhugman

@eliserichards eliserichards force-pushed the 1084-update-appservices branch from 110c387 to 57a4ba4 Compare January 14, 2020 16:57
Copy link
Contributor

@jhugman jhugman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

For a change like this we should bake this for a little while before releasing.

@eliserichards eliserichards merged commit 69d8b36 into master Jan 16, 2020
@rfk
Copy link

rfk commented Jan 21, 2020

Purely for my own curiosity about process, could you please tell me whether you saw the notices in the changelog about how this version of appservices introduces new telemetry and the application should check that it has the appropriate data reviews?

(Lockwise got data-review for these metrics over in Bug 1597895 so there was no action you to take, I'm just curious if you were aware of that change in particular)

@eliserichards eliserichards deleted the 1084-update-appservices branch February 11, 2020 15:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to app services 0.44.0
3 participants