Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix: Android crash when opening notifications #967

Merged
merged 2 commits into from
May 22, 2023
Merged

Fix: Android crash when opening notifications #967

merged 2 commits into from
May 22, 2023

Conversation

C-Flatla
Copy link

@C-Flatla C-Flatla commented May 10, 2023

What's in this PR?

This PR fixes a NullPointerException on Android when app is resumed by opening a notification.

Why was this PR created?

This fixes issues:

Quick summary of the changes you made

  • Wrapped all calls of mNotificationProps.asBundle() in try/catch to prevent app crashes

Any necessary explanation of decisions you made

Picture of cat

cat

* Avoid undefined payloads

* Check for null instead of relying on try/catch

* Remove redundant null check

* Wrap in try/catch instead of null checks
@fotos
Copy link

fotos commented May 11, 2023

Shouldn't we be working on fixing the underlying (root) cause instead of sweeping the problem under the (logging) rug? 🤔

Despite the various problems this library has and the numerous hours I spent debugging (and still do) to get Android to handle notifications correctly in all states (dead/background/foreground), personally, I'm not very fond of the approach taken here.

Copy link
Collaborator

@DanielEliraz DanielEliraz left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@DanielEliraz
Copy link
Collaborator

Shouldn't we be working on fixing the underlying (root) cause instead of sweeping the problem under the (logging) rug? 🤔

Despite the various problems this library has and the numerous hours I spent debugging (and still do) to get Android to handle notifications correctly in all states (dead/background/foreground), personally, I'm not very fond of the approach taken here.

Of course, you're right but a crash is unacceptable

@C-Flatla
Copy link
Author

@DanielEliraz is there anything I need to do to get this merged in? I merged in latest master so it was up to date 😄.

@DanielEliraz
Copy link
Collaborator

@C-Flatla Will be soon in production

@DanielEliraz DanielEliraz merged commit 574d3f3 into wix:master May 22, 2023
DanielEliraz added a commit that referenced this pull request May 22, 2023
@DanielEliraz DanielEliraz mentioned this pull request May 22, 2023
DanielEliraz added a commit that referenced this pull request May 22, 2023
@C-Flatla C-Flatla deleted the fix-android-asbundle-null-pointer-exception branch May 23, 2023 23:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants