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

fix(messaging, android): repair crash handling remote notifications #5236

Merged

Conversation

mikehardy
Copy link
Collaborator

Description

WritableNativeMap has a check to see if it is consumed or not when it is used,
to prevent resolving a Promise in native code then attempting to modify it.

We may use the same WritableNativeMap object twice though when handling deferred
initial notifications. Crash repair is to store a copy so the Maps are distinct

Related issues

Fixes #5231

Release Summary

conventional commit

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

Hard to test - will rely on @xxsnakerxx for reports on whether it happens or not, they appear to have a customer with a reproduction case


Think react-native-firebase is great? Please consider supporting the project with any of the below:

WritableNativeMap has a check to see if it is consumed or not when it is used,
to prevent resolving a Promise in native code then attempting to modify it.

We may use the same WritableNativeMap object twice though when handling deferred
initial notifications. Crash repair is to store a copy so the Maps are distinct
@vercel
Copy link

vercel bot commented Apr 29, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/9K96QYoio9tpV87EPmtzSLWUpa2U
✅ Preview: https://react-native-f-git-mikehardy-issue5231-notification-alre-970816.vercel.app

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Apr 29, 2021
@codecov
Copy link

codecov bot commented Apr 29, 2021

Codecov Report

Merging #5236 (652b47a) into master (b972cb6) will not change coverage.
The diff coverage is n/a.

❗ Current head 652b47a differs from pull request most recent head efe63f3. Consider uploading reports for the commit efe63f3 to get more accurate results

@@           Coverage Diff           @@
##           master    #5236   +/-   ##
=======================================
  Coverage   88.86%   88.86%           
=======================================
  Files         109      109           
  Lines        3743     3743           
  Branches      360      360           
=======================================
  Hits         3326     3326           
  Misses        370      370           
  Partials       47       47           

@mikehardy mikehardy merged commit 6a30d4b into master Apr 29, 2021
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Apr 29, 2021
@mikehardy mikehardy deleted the @mikehardy/issue5231-notification-already-consumed branch April 29, 2021 21:17
mikehardy added a commit that referenced this pull request May 6, 2021
… 0.61.0

#5236 accidentally required a symbol defined here facebook/react-native@ac7ec46 which was not released until react-native 0.61
mikehardy added a commit that referenced this pull request May 6, 2021
this should be the inline equivalent of the rn61 symbol I added
in #5236 - accidentally breaking react-native 0.60 compatibility
mikehardy added a commit that referenced this pull request May 6, 2021
this should be the inline equivalent of the rn61 symbol I added
in #5236 - accidentally breaking react-native 0.60 compatibility
mikehardy added a commit that referenced this pull request May 12, 2021
this should be the inline equivalent of the rn61 symbol I added
in #5236 - accidentally breaking react-native 0.60 compatibility
mikehardy added a commit that referenced this pull request May 12, 2021
this should be the inline equivalent of the rn61 symbol I added
in #5236 - accidentally breaking react-native 0.60 compatibility
mikehardy added a commit that referenced this pull request May 12, 2021
this should be the inline equivalent of the rn61 symbol I added
in #5236 - accidentally breaking react-native 0.60 compatibility
hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this pull request Jul 13, 2021
this should be the inline equivalent of the rn61 symbol I added
in invertase#5236 - accidentally breaking react-native 0.60 compatibility
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.

[🐛] getInitialNotification - Map already consumed
1 participant