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

Firebase Messaging iOS SDK version 7.0 will be a breaking change where the SDK will no longer support iOS Direct Channel API #3674

Closed
khalilTN opened this issue May 22, 2020 · 21 comments · Fixed by #3733

Comments

@khalilTN
Copy link

Hi, we have received this message from Firebase

Firebase Messaging iOS SDK version 7.0 will be a breaking change where the SDK will no longer support iOS Direct Channel API

We are on Firebase/Core 6.13.0 and FirebaseMessaging (~> 4.1.9)
Is iOS Direct Channel used in this version ?
Can we upgrade to 7.0 ?

Thank you

@AlexanderKaplunsky
Copy link

Hi, we are using react-native-firebase v5.5.6. Is it possible to discontinue use of the iOS Direct Channel API and use an alternative such as FCM’s APNs interface for downstream data message delivery?

@javierbustamante
Copy link

Hi!!!
We have received the same message from firebase so we are very very interested in the issue too.
We just use FCM for iOS Push Messaging (besides analytics and crashlytics) with the v5 of react-native.firebase, so, I'm not sure if that deprecation may affect our project.
Thanks!!!

@mikehardy
Copy link
Collaborator

react-native-firebase v5 is not supported. Nothing I say will be directed at v5

For v6, what exactly is the problem? I'm not seeing there is an actual problem stated, just questions. This is a community site correct? We're all volunteers? Someone please do the research to answer these questions:

  • is direct channel API used? If so under what conditions?
    -- I think if you are using vanilla FCM features you're probably fine but if you're concerned you should probably go and find the docs and check right?
  • if you are using the direct channel API somehow, what is your migration path, meaning what specific APIs were you accessing and what should you access?

I see 21 votes here and not one person has checked upstream and reported back? We have to help ourselves here - as a community we/you need to understand that being a maintainer basically means you have no time available, so in cases were there is a big pool of people interested, someone should be able to lend a hand and help everyone.

Any takers?

@dirkpostma
Copy link

dirkpostma commented May 26, 2020

According to the documentation, by default, shouldEstablishDirectChannel is set to No in the SDK.

In v6, I can't find any code that sets this property to true, so no issue there, I think. Not sure why llaine gets the message. May be I overlooked something. Can anyone else check?

in v5, shouldEstablishDirectChannel seems to be always true, so this will be an issue for v5 users.

// In RNFirebaseMessaging.m 

// Establish Firebase managed data channel
[FIRMessaging messaging].shouldEstablishDirectChannel = YES;

@vgm8
Copy link

vgm8 commented May 26, 2020

In V6 I see that flag set to true in this file: https://github.com/invertase/react-native-firebase/blob/master/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BFIRMessagingDelegate.m

[FIRMessaging messaging].shouldEstablishDirectChannel = YES;

@dirkpostma
Copy link

I was wrong:

[FIRMessaging messaging].shouldEstablishDirectChannel = YES;

Here it IS set ti true.. Ignore my previous message.

@Pyroboomka
Copy link

Sorry for being out of the loop, but what happened to the idea of making notifications paid package after 6.0.0?
Because that was the case on staying on 5.x.x for me when the version 6 was released.

@tedcurrent
Copy link

Is the shouldEstablishDirectChannel actually required in v5 or v6? Does it just happen to be set to YES by default?

@mikehardy
Copy link
Collaborator

@tedcurrent that's the big question - it appears based on research (thanks!) that it toggled on, but is it actually an issue? Has anyone tested their messaging behavior after toggling it off? I think that it will only be an issue for people that have not correctly set up their APNS channel - in the official Google Firebase "troubleshooting / understanding message delivery" video they mention that if you see message delivery when app is in foreground, but not in background, it could be that direct channel is working but APNS is not configured, so it fails. So I always understood direct channel to just be a "more efficient" mechanism but also a troubleshooting trap. If they remove direct channel then there will be more consistent behavior (delivery works or not, APNS every time) which is actually a plus, but means only people with incorrect configs will notice

In that sense, I'm guessing (needs validation by anyone that wants to give it a shot) that if you toggle direct channel off on app boot, in a correctly configured (read as: APNS works / delivery already works in background) app, this is a "no change" change.

@Pyroboomka -> http://notifee.app/

@DGA-Native
Copy link

DGA-Native commented May 27, 2020

Hello, world!

I would like to emphasize that in v5, v6 but ALSO in v7 the value is the same [FIRMessaging messaging] .shouldEstablishDirectChannel = YES;

On the other hand, there is a thread from 2017 that poses the reverse scenario, from [FIRMessaging messaging] .shouldEstablishDirectChannel = NO to YES. https://forums.xamarin.com/discussion/103978/data-messages-broken-with-the-new-firebase-update

I think the change in this parameter is not trivial, I at least do not understand how after an email alerting that it is deprecated, it follows YES and also it is not clear how the behavior will be if finally the value remains in v7.

Thank you very much and greetings.

@mikehardy
Copy link
Collaborator

Hi @DGA-Native

I think the change in this parameter is not trivial,

Have you tried it to see how your app behaves?

@MrLoh
Copy link
Contributor

MrLoh commented Jun 2, 2020

@mikehardy I gave it a try in my project and all notifications still arrive on iOS, I created a PR #3733 with the change.

@mikehardy
Copy link
Collaborator

The testing is what takes so much time, thank you for trying the change and verifying it still worked for you, that PR is much appreciated!

@MrLoh
Copy link
Contributor

MrLoh commented Jun 3, 2020

Thank you for providing this amazing project. If there are further types of notifications to test out, let me know on the PR.

@graphee-gabriel
Copy link

Hello! Any update on this issue's status?
Until the PR is merged would anyone have a temporary solution for us to safely disable direct channel api?
Thanks for the support :-) !

@MrLoh
Copy link
Contributor

MrLoh commented Jun 12, 2020

You cannot disable it in your app unless you fork the SDK, but just don't send messages via direct channels and you will be future proof.

@graphee-gabriel
Copy link

graphee-gabriel commented Jun 12, 2020

Hmmm I might have missed a step, i'm sending push notifications through the admin sdk:

const message = {
    tokens: batchOfTokens,
    notification: {
      title,
      body,
    },
    data,
    android: {
      ttl: ttlInSecond * 1000,
    },
    apns: {
      headers: {
        'apns-expiration': apnsExpiration.toString(),
      },
    },
    webpush: {
      headers: {
        'TTL': ttlInSecond.toString(),
      },
    },
  };
};
admin.messaging().sendMulticast(message)

I'm not sure where i'm making any reference to the direct channel api server side.

@MrLoh
Copy link
Contributor

MrLoh commented Jun 12, 2020

@graphee-gabriel than this whole issue is not an issue for you in any way, it only matters if you were sending data only messages to iOS as far as I understand.

@graphee-gabriel
Copy link

Hmmm ok, even though i did receive the firebase email saying:

You are an owner of the following Firebase project(s) that uses Firebase Messaging iOS Direct Channel API:

  • {project-name}

You think my project is coming up as a false positive, or I potentially missed something somewhere? Thank you once more for your help!

@MrLoh
Copy link
Contributor

MrLoh commented Jun 12, 2020

@graphee-gabriel no its not a false positive this library enables it by default, you are not using every aspect of this library though so it shouldn't matter. If you want to be sure, then please look into the native code to understand where this is set and test your own project.

@morneluckybeard
Copy link

I too can confirm that my messaging is working fine while setting [FIRMessaging messaging].shouldEstablishDirectChannel = NO in AppDelegate.m

I'm using v5.6.0

Salakar added a commit that referenced this issue Jul 14, 2020
* set shouldEstablishDirectChannel to NO

* tests: make `onMessage` test Android only

This test will no longer function on simulators as a direct message channel is no longer established.

Co-authored-by: Mike Diarmid <[email protected]>
hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this issue Jul 13, 2021
…tase#3733)

* set shouldEstablishDirectChannel to NO

* tests: make `onMessage` test Android only

This test will no longer function on simulators as a direct message channel is no longer established.

Co-authored-by: Mike Diarmid <[email protected]>
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 a pull request may close this issue.