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

[Pushnotifications] Add error event #2336

Closed
wants to merge 3 commits into from
Closed

[Pushnotifications] Add error event #2336

wants to merge 3 commits into from

Conversation

DannyvanderJagt
Copy link
Contributor

This PR will add an error event to PushNotificationIOS.

How it works
When something goes wrong with the Push notifications, for example running on a simulator or when the certificates aren't configured the right way, IOS will create an error and pass it to didFailToRegisterForRemoteNotificationsWithError. This error consists out of an key and a message. This error is passed down from IOS to Js and when there is an error event registered in JS it will pass the error. When there isn't an error event registered nothing will happen.

Why is this useful?
At the moment PushNotificationIOS fails silently and there is no way of telling if something went wrong. Therefore some people are having trouble to get push notifications working in react-native.

How to use
The error event can be used just like the register and the notification event.
The error callback will give the error message and the error key. Both are the original IOS error.

// Javascript:
PushNotificationIOS.addEventListener('error', function(message, key){
   console.log('An error occurred: ', message);
});
PushNotificationIOS.addEventListener('register', function(token){
   console.log('You are registered and the device token is: ',token)
});
PushNotificationIOS.requestPermissions();

And add this to the AppDelegate.m besides the normal pushnotifications code:

- (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error
{
  [RCTPushNotificationManager application:application didFailToRegisterForRemoteNotificationsWithError:error];
}

Similar PR
A similar solution PR exists: #1019
This PR uses a callback as a parameter in requestPermissions and it will give you a token or an error.
I prefer the solution is this PR because it is:

  • more in line with the react-native Api.
  • less potential spaghetti code
  • I have used this in an proof of concept app and I prefer the event over the callback.

References
This PR was part of #1979 and it is discussed here: #1613.

This event should be added to the docs. Can anyone tell me how accomplish this?

Please let me know what you think.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Aug 14, 2015
@PhilippKrone
Copy link
Contributor

@ide Do you have any information here about whether this is going to get merged or any chance you could review this?

@anthonywebb
Copy link

I need this as well, hoping push notifications can be properly supported in react, seems kind of half baked and broken in its current state.

@DannyvanderJagt
Copy link
Contributor Author

@PhilippKrone They are a little busy with the android release at the moment I think. But it would be nice if this could be merged/reviewed. @ide Let us/me know if we can help you.

@anthonywebb push notifications are working in React-Native for IOS when you apply the fix from this PR. I have written a tutorial about How to use push notifications in React-Native., it also includes the instructions to apply that fix to your project. Besides the push notifications, there are some issues and PRs for local notifications in IOS but nothing is merged yet (as far I know).

@justinmakaila
Copy link

This is great. I'd love for this to be integrated. Any estimates?

@JonnyBGod
Copy link

+1

@DannyvanderJagt
Copy link
Contributor Author

@justinmakaila There is not estimate. The React-Native team is very busy at the moment. I will update and rebase this PR this weekend to get it up to date.

@jadsonlourenco
Copy link

+1

@satya164
Copy link
Contributor

@DannyvanderJagt Any updates on this?

@basketofsoftkittens
Copy link

+1

@satya164
Copy link
Contributor

Closing since no activity on the PR. let's re-open if you wanna work on it again.

@satya164 satya164 closed this Jan 26, 2016
@niftylettuce
Copy link
Contributor

Can we get this in?

@niftylettuce
Copy link
Contributor

If anyone else needs something temporary, add this to your AppDelegate.m file:

// listen for errors (since iOS simulator doesn't allow push notification)
// <https://github.com/facebook/react-native/pull/2336>
// <http://stackoverflow.com/a/31707874>
- (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error
{
  RCTLogWarn(@"failed to register for notifications %@", error);
  //[RCTPushNotificationManager application:application didFailToRegisterForRemoteNotificationsWithError:error];
}

@DannyvanderJagt
Copy link
Contributor Author

@niftylettuce Yes of course we can get this in. I will revisit this PR this weekend and hopefully it will get merged soon.

I'm sorry for the big delay but I just got slammed with work the last few months.

@DannyvanderJagt
Copy link
Contributor Author

I couldn't get the old branch updated without destroying this PR. I forked the master of react-native today and added back and updated all the code of the this PR.

See: #7694

@niftylettuce Thank you for reminding me :)

facebook-github-bot pushed a commit that referenced this pull request Sep 6, 2016
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
mikelambert pushed a commit to mikelambert/react-native that referenced this pull request Sep 29, 2016
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants