[Push notifications] Add error event #7694
Conversation
|
By analyzing the blame information on this pull request, we identified @slycoder and @nicklockwood to be potential reviewers. |
ghost
added
GH Review: review-needed
CLA Signed
| - (void)handleRemoteNotificationRegisteredError:(NSNotification *)notification | ||
| { | ||
| [_bridge.eventDispatcher sendDeviceEventWithName:@"remoteNotificationsRegisteredError" | ||
| body:[notification userInfo]]; |
There was a problem hiding this comment.
Judging by the preceding notification handlers, dot-notation is preferred: notification.userInfo.
|
@DannyvanderJagt Did a bit of review, otherwise it looks good to me. Hope it helps. |
|
@alloy Thank you! I will update the PR in a few minutes. |
|
@nicklockwood Code-wise this seems good to me 👍 |
|
@DannyvanderJagt updated the pull request. |
|
Hey, just noticed this still hasn't landed yet. Is there anything that I can do to help push this PR along? |
|
We are waiting for a review from a contributor I believe. I think that there are busy :) |
|
I think this will need some rebasing due to the event emitter changes, but I'll see what I can do. |
|
@facebook-github-bot shipit |
|
@nicklockwood Thank you! Let me know if there is anything i can do. |
| @@ -135,6 +140,15 @@ + (void)didReceiveLocalNotification:(UILocalNotification *)notification | |||
| userInfo:details]; | |||
| } | |||
|
|
|||
|
|
|||
|
|
|||
| + (void)didFailToRegisterForRemoteNotificationsWithError:(NSError *)error | |||
There was a problem hiding this comment.
Can you the update the documentation in
to mention this method.
|
It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @nicklockwood as a potential reviewer. Could you take a look please or cc someone with more context? |
|
@DannyvanderJagt looks like this could use a rebase off of master; it has some conflicts w/ the new event emitter code |
|
@nevir @nicklockwood I will rebase this PR in the next few days. |
|
I see this pull request was closed. Is there a follow-up PR? If yes, it would be good to link to it from here. |
|
Just opened an updated version of this PR via #9650 Note that it has a few differences, if you were previously depending on this patch:
|
Summary: This is an updated version of #2336 and #7694. --- This adds a `registrationError` event that is emitted by `PushNotificationIOS` whenever an application receives a registration error from APNS (APNS service failure, running on simulator, etc). This event fires to the exclusion of the `register` event (and vice versa). **How to use** Add the following to your `AppDelegate.m`: ```obj-c - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error { [RCTPushNotificationManager didFailToRegisterForRemoteNotificationsWithError:error]; } ``` And register an event handler for the event: ```js PushNotificationIOS.addEventListener('registrationError', function({ message, code }) { // Complete your registration process in error. }); ``` **Test plan** Added support for this event (and `register`) to UIExplorer as a proof of concept. Navigating to the push notifications example on a simulator is an easy way to reproduce this e Closes #9650 Differential Revision: D3822142 Pulled By: javache fbshipit-source-id: a15ed8941b74dc3eed2c44c658deccbcaf39ce3d
Summary: This is an updated version of facebook#2336 and facebook#7694. --- This adds a `registrationError` event that is emitted by `PushNotificationIOS` whenever an application receives a registration error from APNS (APNS service failure, running on simulator, etc). This event fires to the exclusion of the `register` event (and vice versa). **How to use** Add the following to your `AppDelegate.m`: ```obj-c - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error { [RCTPushNotificationManager didFailToRegisterForRemoteNotificationsWithError:error]; } ``` And register an event handler for the event: ```js PushNotificationIOS.addEventListener('registrationError', function({ message, code }) { // Complete your registration process in error. }); ``` **Test plan** Added support for this event (and `register`) to UIExplorer as a proof of concept. Navigating to the push notifications example on a simulator is an easy way to reproduce this e Closes facebook#9650 Differential Revision: D3822142 Pulled By: javache fbshipit-source-id: a15ed8941b74dc3eed2c44c658deccbcaf39ce3d
|
@nevir Thank you for picking up this PR! |
This PR will add an
errorevent toPushNotificationIOS.How does this work?
The errors from
didFailToRegisterForRemoteNotificationsWithErrorwill be parsed to theerrorevent in javascript.Why is this useful?
At the moment PushNotificationIOS fails silently and there is no way of telling if something went wrong.
It would be very helpful to see the errors for debugging and in-app error handling.
How to use
The error event can be used just like the register and the notification events.
The error callback will give the original error object from IOS.
To use the
errorevent in javascript add these line to yourAppDelegate.m:PR
This is an new updated version of my 7 month closed old PR: #2336. I couldn't get the old branch updated without destroying the PR. I forked the master of react-native today and added back and updated all the code of the old PR.
TODO: Docs
This event should be added to the docs and the website. Can someone tell me how I can accomplish this?
TODO: Review
Objective-C is not a language I use often. It would be appreciated if someone could review this PR :)