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

Do not crash when the main activity is unavailable #399

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

swansontec
Copy link

Summary

The React Native application context has a method for obtaining the current Android activity, but this sometimes returns null. We believe this happens due to a race condition between the Javascript engine and the native code, but we aren't completely sure. What we do know is that this situation happens in the wild, since it shows up in our Bugsnag reports.

The fix is the leave the BlurView native component uninitialized in these cases. This prevents it from rendering anything, but that's preferable to crashing the whole app.

Screen Shot 2020-09-29 at 8 31 19 AM

Test Plan

Since we don't have a reproduction case for the crash, we intend to deploy these changes in the next release of our App, and then see if our Bugsnag reports are any cleaner.

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

The React Native application context has a method for obtaining the current Android activity, but this sometimes returns null. We believe this happens due to a race condition between the Javascript engine and the native code, but we aren't completely sure. What we do know is that this situation happens in the wild, since it shows up in our Bugsnag reports.

The fix is the leave the BlurView native component uninitialized in these cases. This prevents it from rendering anything, but that's preferable to crashing the whole app.
@ericchan3721
Copy link

Seems you're missing import the Activity class in the BlurViewManager.java

@swansontec
Copy link
Author

Oops! Something must have gone terribly wrong when I first submitted the PR, but I put back the missing line (which we definitely have in our app).

We have been running this patch in production for the past few months, and it seems to have cleared up the issue entirely - we no longer have any instances of this crash in our Bugsnag reports.

@mrousavy
Copy link

I don't think this is an appropriate fix. The createViewInstance function should not be called when the Activity is null. Have you tried using the AndroidX Activity getter? I'm also pretty sure this will change with Fabric.

@swansontec
Copy link
Author

I don't think this is an appropriate fix. The createViewInstance function should not be called when the Activity is null. Have you tried using the AndroidX Activity getter? I'm also pretty sure this will change with Fabric.

I agree! This is almost certainly a bug within React Native itself, and so the correct fix should go into React Native itself. Without a clear reproduction case, though, fixing that is rather difficult. Hence, this is just a workaround for the symptom, to avoid app death in the wild.

I don't know what the "AndroidX Activity getter" is, but I would be open to trying that.

@Titozzz
Copy link
Collaborator

Titozzz commented Jul 21, 2022

Is this still valid ?

@swansontec
Copy link
Author

Is this still valid ?

It is unclear. Our app has been shipping with this patch for years, and we no longer have any blur-related crashes in our Bugsnag tracker. So the patch is not hurtful, at the very least.

It is unclear if the upstream React Native platform has fixed that bug that causes this crash. We just don't have any data, since the patch makes the bug irrelevant.

@swansontec swansontec closed this Jul 21, 2022
@swansontec swansontec reopened this Jul 21, 2022
@Titozzz
Copy link
Collaborator

Titozzz commented Aug 2, 2022

If you want to fix conflicts I can merge

@shaverda
Copy link

shaverda commented Dec 8, 2022

Is this still valid ?

It is unclear. Our app has been shipping with this patch for years, and we no longer have any blur-related crashes in our Bugsnag tracker. So the patch is not hurtful, at the very least.

It is unclear if the upstream React Native platform has fixed that bug that causes this crash. We just don't have any data, since the patch makes the bug irrelevant.

It appears to be still valid for our app; we are having the same crash with no reliable way to reproduce. RN 0.69.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants