Get rid of StatusService. #7334
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (42)
|
|
|
||
| defaultConfig { | ||
| minSdkVersion 18 | ||
| targetSdkVersion 26 | ||
| minSdkVersion 23 |
There was a problem hiding this comment.
Does this mean that the lowest Android version we support is 6.0?
There was a problem hiding this comment.
The app itself already requires minSdkVersion 23, so to make react-native-status do that, it's no problem.
|
@Swader hey! since you were along the people who were able to reproduce this issue quite easily, would you mind checking if this PR actually solves it? Keep in mind, that that will most probably overwrite your current version of the app. It is based on our nightlies, so it should be pretty solid, but you can always lose data with it... So, do it only if you have your account seed phrase backed up and keep in mind that you might lose your contacts there... |
|
@mandrigin I would love to but need help on how to do this as I don't know how to build and test a PR version of Status for Android. Any guide I can reference? |
|
@Swader if you are okay with all the precautions, then you can just download and install this APK (the latest one for android on the table of builds: But again, I want to reiterate that:
Or, if you are using nightlies, maybe you can just wait for the next nightly and this fix will be included in it. |
|
Unfortunately, right now it is impossible to install PR builds aside the release, but this PR will fix it: #7287 |
|
Android just gives me "app not installed" after downloading and clicking Install and confirming an update process. No other error or explanation. |
|
Oh, thanks to Android for not being helpful. |
|
I am currently on last night's nightly, installed without a problem just an hour ago. |
|
@Swader any chance you have |
|
I do now. What do I need to do? |
|
|
|
|
@Swader ah, |
|
@Swader try installing this one: https://status-im-prs.ams3.digitaloceanspaces.com/StatusIm-190125-114343-6e4d3c-pr.apk |
|
No effect. Or should I say same effect, different message. |
|
@Swader thats... interesting. Can you tell me the exact steps you used to reproduce it and the model/maker of your Android device as well as if the firmware installed (stock or some open-source installed). Also, |
|
|
|
it really looks like that this is an Android 9 bug, many people are having this issue: https://issuetracker.google.com/issues/113122354 |
100% of end-end tests have passedPassed tests (58)Click to expand
|
mandrigin
force-pushed
the
bugfix/7198-crash-android-
branch
from
b471217 to
72b6ce5
Compare
|
@annadanchenko @rasom @pombeirp I ended up getting rid of |
mandrigin
changed the title
mandrigin
changed the title
pedropombeiro
dismissed stale reviews from rasom and themself
Approach to fix has changed
|
it should also fix #7281 |
|
@Swader can you try this build and check if you can reproduce the issue? https://status-im-prs.ams3.digitaloceanspaces.com/StatusIm-190125-144936-59485e-pr.apk |
|
OnePlus 6T, stock. Reproduce: Go anywhere in the app (in this case localethereum) that isn't the home screen and then let the phone go to sleep. Wake 30 seconds later and it will always appear. Relevant: #7198 Logcat: Testing new build in a few mins. |
|
Problem resolved, thank you :) |
|
@Swader awesome! Btw, looking on your logcat. I don't know what Google did with Android 9.0 but services are really broken:
|
|
@Swader thanks a lot for testing the app! |
mandrigin
force-pushed
the
bugfix/7198-crash-android-
branch
from
59485ee to
398267e
Compare
|
history rewrite to sign the commit and get autotests to run :) |
100% of end-end tests have passedPassed tests (58)Click to expand
|
modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusService.java
Outdated
Show resolved
Hide resolved
modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusModule.java
Outdated
Show resolved
Hide resolved
|
Update: can reproduce on nightly, so not introduced by this PR @mandrigin sometimes there is an error or crash upon logout (investigating exact case when it happens and trying to reproduce also on nightly), account has public, group and 1:1 chats and performed some transactions. Pre-selected mailserver is used (mail-03.ac-cn-hongkong-c.eth.beta), develop mode is off. these steps can lead to the issue: preselect mailserver and logout/login again
Another set of smaller logs after issue is reproduced: |
mandrigin
force-pushed
the
bugfix/7198-crash-android-
branch
from
d688d2d to
04579f1
Compare
StatusService was only used to handle `signalEvent:` from status-go. This commit simplifies this interaction and getting rid of the service and all the problems that come with it. Signed-off-by: Igor Mandrigin <i@mandrigin.ru>
mandrigin
force-pushed
the
bugfix/7198-crash-android-
branch
from
04579f1 to
fafecfe
Compare
Android 9 somehow changed the restrictions of starting background services in
Activity#onResume, it isn't allowed anymore. I'm not exactly sure if that is intended or a bug.UPD: it's an Android bug:
This change delays starting the service up until the app is in foreground, by using lifecycle listeners (see this article to read more on them).That is what I used as an example of this workaround: mapbox/mapbox-events-android#157Okay, StatusService was only used as a weird layer between status-go to pass
signalEvent:. I simplified it in a way that is still compatible with old status-go (no class rename), but it works completely differently.In a nutshell, we just use a singleton class that either gets initialised when the
StatusModuleis there or when JNI is looking up for an instance. Then it just passes all thesignalEvent:calls toStatusModule.That way we don't need for service to initialise, so I removed all mentions of it and Android works roughly the same as iOS now.
fixes #7198
fixes #7281
status: ready