search

aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.42k stars 2.12k forks source link

Can't update optOut for AWS Pinpoint endpoint without address (APNs device token) #13272

Closed victoravr closed 4 months ago

victoravr commented 5 months ago

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

Push Notifications

Amplify Version

v6

Amplify Categories

notifications

Backend

CDK

Environment information

``` System: OS: macOS 14.4.1 CPU: (8) arm64 Apple M2 Memory: 58.22 MB / 24.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 21.7.3 - /opt/homebrew/bin/node Yarn: 1.22.22 - /opt/homebrew/bin/yarn npm: 10.5.0 - /opt/homebrew/bin/npm Watchman: 2024.04.15.00 - /opt/homebrew/bin/watchman Browsers: Chrome: 124.0.6367.62 Edge: 124.0.2478.51 Safari: 17.4.1 npmPackages: @aws-amplify/react-native: ^1.0.28 => 1.0.28 @aws-amplify/rtn-push-notification: ^1.2.28 => 1.2.28 @aws-amplify/ui-react-native: ^2.1.5 => 2.1.5 @aws-crypto/sha256-js: ^5.2.0 => 5.2.0 (3.0.0) @babel/core: ^7.23.6 => 7.24.0 @babel/preset-env: ^7.23.6 => 7.24.0 @babel/preset-react: ^7.23.3 => 7.23.3 @babel/runtime: ^7.23.6 => 7.24.0 @react-native-async-storage/async-storage: ^1.23.1 => 1.23.1 @react-native-community/netinfo: ^11.2.1 => 11.3.1 @react-native-community/slider: ^4.5.0 => 4.5.0 @react-native-picker/picker: ^2.6.1 => 2.6.1 @react-native/babel-preset: 0.73.21 => 0.73.21 @react-native/eslint-config: 0.73.2 => 0.73.2 @react-native/metro-config: 0.73.5 => 0.73.5 @react-native/typescript-config: 0.73.1 => 0.73.1 @react-navigation/bottom-tabs: ^6.5.11 => 6.5.16 @react-navigation/drawer: ^6.6.6 => 6.6.11 @react-navigation/native: ^6.1.9 => 6.1.14 @react-navigation/stack: ^6.3.20 => 6.3.25 @reduxjs/toolkit: ^2.0.1 => 2.2.1 @reduxjs/toolkit-query: 1.0.0 @reduxjs/toolkit-query-react: 1.0.0 @reduxjs/toolkit-react: 1.0.0 @rneui/base: ^4.0.0-rc.7 => 4.0.0-rc.7 @rneui/themed: ^4.0.0-rc.8 => 4.0.0-rc.8 @rnx-kit/align-deps: ^2.3.1 => 2.3.4 @tanstack/query-codemods: 4.24.3 @tanstack/react-query: ^5.28.4 => 5.28.4 @tsconfig/react-native: ^3.0.2 => 3.0.3 @types/react: ^18.2.45 => 18.2.61 @types/react-redux: ^7.1.33 => 7.1.33 @types/react-test-renderer: ^18.0.7 => 18.0.7 HelloWorld: 0.0.1 aws-amplify: ^6.0.28 => 6.0.28 aws-amplify/adapter-core: undefined () aws-amplify/analytics: undefined () aws-amplify/analytics/kinesis: undefined () aws-amplify/analytics/kinesis-firehose: undefined () aws-amplify/analytics/personalize: undefined () aws-amplify/analytics/pinpoint: undefined () aws-amplify/api: undefined () aws-amplify/api/server: undefined () aws-amplify/auth: undefined () aws-amplify/auth/cognito: undefined () aws-amplify/auth/cognito/server: undefined () aws-amplify/auth/enable-oauth-listener: undefined () aws-amplify/auth/server: undefined () aws-amplify/data: undefined () aws-amplify/data/server: undefined () aws-amplify/datastore: undefined () aws-amplify/in-app-messaging: undefined () aws-amplify/in-app-messaging/pinpoint: undefined () aws-amplify/push-notifications: undefined () aws-amplify/push-notifications/pinpoint: undefined () aws-amplify/storage: undefined () aws-amplify/storage/s3: undefined () aws-amplify/storage/s3/server: undefined () aws-amplify/storage/server: undefined () aws-amplify/utils: undefined () babel-jest: ^29.7.0 => 29.7.0 babel-loader: ^9.1.3 => 9.1.3 detox: ^20.19.5 => 20.19.5 eslint: ^8.56.0 => 8.57.0 formik: ^2.4.5 => 2.4.5 ini: ^4.1.1 => 4.1.1 (1.3.8) inquirer: ^9.2.12 => 9.2.15 jest: ^29.7.0 => 29.7.0 metro-react-native-babel-preset: ^0.77.0 => 0.77.0 prettier: ^3.1.1 => 3.2.5 react: 18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 react-native: ^0.73.6 => 0.73.6 react-native-awesome-slider: ^2.4.6 => 2.5.1 react-native-codegen: ^0.70.7 => 0.70.7 react-native-fast-image: ^8.6.3 => 8.6.3 react-native-gesture-handler: ^2.14.0 => 2.15.0 react-native-get-random-values: ^1.11.0 => 1.11.0 react-native-image-crop-picker: ^0.40.2 => 0.40.3 react-native-modal: ^13.0.1 => 13.0.1 react-native-picker-select: ^9.0.1 => 9.0.1 react-native-purchases: ^7.5.1 => 7.22.0 react-native-reanimated: ^3.6.1 => 3.7.2 react-native-safe-area-context: ^4.8.2 => 4.9.0 react-native-screens: ^3.29.0 => 3.29.0 react-native-toast-message: ^2.2.0 => 2.2.0 react-native-url-polyfill: ^2.0.0 => 2.0.0 react-native-vector-icons: ^10.0.3 => 10.0.3 react-redux: ^9.0.4 => 9.1.0 react-test-renderer: 18.2.0 => 18.2.0 redux-persist: ^6.0.0 => 6.0.0 redux-persist/integration/react: undefined () redux-thunk: ^3.1.0 => 3.1.0 typescript: ^5.3.3 => 5.3.3 yup: ^1.3.3 => 1.3.3 npmGlobalPackages: @devicecloud.dev/dcd: 0.0.5 detox-cli: 20.0.0 ios-deploy: 1.12.2 npm: 10.5.0 ```

Describe the bug

Can't update optOut for AWS Pinpoint endpoint without address (APNs device token)

Expected behavior

Should be able to update optOut (e.g. set to 'NONE') for AWS Pinpoint endpoint without address (APNs device token)

Reproduction steps

call identifyUser() after user logs in. If no device token provided along with optOut option as address, console warning appears as in screenshot.

  options: {
    address: deviceToken,
    optOut: 'NONE'
  },

Code Snippet

import {
  identifyUser,
} from 'aws-amplify/push-notifications';
...

const identifyUserInput =  {
      userId: userId,
      userProfile: {
        customProperties: userInfo,
      },
      options: {
        optOut: 'NONE'
      },
    }

await identifyUser(identifyUserInput);

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

iPhone 15 Pro + Simulator

Mobile Operating System

iOS 17.4

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

image
cwomack commented 5 months ago

Hello, @victoravr and thank you for opening this issue. Can you confirm if you also see this happen when you set optOut to ALL as well? Also, are you calling initializePushNotifications() before your identifyUser() call?

This does seem to be a bug and we'll investigate this as such. The address should not be required per Pinpoint's documentation.

cshfang commented 5 months ago

Hi @victoravr. As @cwomack mentioned, we are currently treating this as a bug, however I am unable to reproduce the error via the steps below:

  1. Start my RN app on iOS simulator
  2. Call identifyUser with optOut: 'ALL'
  3. Call identifyUser with optOut: 'NONE'

Could you please share more of your use case with us so we can better reproduce the circumstances around which you are seeing the issue? It would also be helpful if you could use Flipper or something similar to inspect the actual network calls being made? In particular - we want to be sure that the endpointId of the calls are consistent between calls E.g.

// On app start, request is made automatically to 
// https://pinpoint.<region>.amazonaws.com/v1/apps/123/endpoints/456
{
  RequestId: "abc123",
  ...
  Address: "my-token-string",
  ...
}
// On identifyUser call, request is made to (same endpoint)
// https://pinpoint.<region>.amazonaws.com/v1/apps/123/endpoints/456
{
  RequestId: "xyz789",
  ...
  OptOut: "ALL",
  ...
}
victoravr commented 5 months ago

Hello, @victoravr and thank you for opening this issue. Can you confirm if you also see this happen when you set optOut to ALL as well? Also, are you calling initializePushNotifications() before your identifyUser() call?

Sorry, haven't tried setting optOut to ALL yet. Will try it soon (need to set it to ALL for the endpoint upon user sign-out I guess) and will report back.

And yes, initializePushNotifications() is called before identifyUser() call.

victoravr commented 5 months ago

Hi @victoravr. As @cwomack mentioned, we are currently treating this as a bug, however I am unable to reproduce the error via the steps below:

  1. Start my RN app on iOS simulator
  2. Call identifyUser with optOut: 'ALL'
  3. Call identifyUser with optOut: 'NONE'

Could you please share more of your use case with us so we can better reproduce the circumstances around which you are seeing the issue? It would also be helpful if you could use Flipper or something similar to inspect the actual network calls being made? In particular - we want to be sure that the endpointId of the calls are consistent between calls E.g.

// On app start, request is made automatically to 
// https://pinpoint.<region>.amazonaws.com/v1/apps/123/endpoints/456
{
  RequestId: "abc123",
  ...
  Address: "my-token-string",
  ...
}
// On identifyUser call, request is made to (same endpoint)
// https://pinpoint.<region>.amazonaws.com/v1/apps/123/endpoints/456
{
  RequestId: "xyz789",
  ...
  OptOut: "ALL",
  ...
}

Hmm, could you try to reproduce the error on the physical device, if possible? I have noticed this error while testing on iPhone Xr with iOS 17.4.1.

victoravr commented 5 months ago

OK, I have updated identifyUser() call to set optOut to 'ALL' upon user sign-out.

During testing this time I can't reproduce the reported error - neither for 'NONE', nor for 'ALL' optOut without address.

If you can't reproduce it on the physical device too, happy to close this issue - maybe it was a bug in my code.

Thanks!

cwomack commented 5 months ago

Appreciate the confirmation, @victoravr. We haven't been able to reproduce it on out side either following the documented flows. We'll close this issue out for now, but feel free to reply back if you happen to find a way to reproduce it reliably again!

victoravr commented 5 months ago

Hi @cwomack and @cshfang,

Getting this error again! :( This time I've installed Reactotron for debugging. Here's API request:

RequestId:4fd0c28e-4af5-46dd-a795-aeb765af7cc2
EffectiveDate:2024-04-28T00:53:07.680Z
ChannelType:APNS_SANDBOX
Attributes:
AppVersion:ios/17.4
Make:null
Model:null
ModelVersion:17.4
Platform:ios
Location:
OptOut:NONE
UserId:c9ae5488-2001-70f4-13d8-24ff71a0ff45
image

And response: Message Missing Address

image image

Is it a bug?

cshfang commented 5 months ago

Hmm is this the first network call to Pinpoint being made by the application? When initializing push notifications, there should be a call made automatically to bootstrap the endpoint with the token (address).

Would it be possible to add a onTokenReceived handler to check if the token is being received? How early on is the initializePushNotifications call being made in the lifecycle of your application?

What is supposed to happen is:

  1. When remote notification is registered, a token received native event is queued
  2. When initializePushNotifications is called, listeners for all native events are wired up, flushing the aforementioned queue
  3. The token received native event is picked up by the JS side
  4. An endpoint is created/updated with the token as the address

It seems like somehow step 4 either did not trigger or did not complete in time 🤔 so it would be helpful to know if there are any other calls being made by the library at all..

victoravr commented 5 months ago

Thanks for prompt reply, @cshfang!

Hmm is this the first network call to Pinpoint being made by the application? When initializing push notifications, there should be a call made automatically to bootstrap the endpoint with the token (address).

Yes, looks like it was the first network call to Pinpoint made by the app since launch.

Would it be possible to add a onTokenReceived handler to check if the token is being received? How early on is the initializePushNotifications call being made in the lifecycle of your application?

I've got onTokenReceived used inside useEffect of the React component, as below:

import React, {useEffect} from 'react';
import {
  onTokenReceived,
  OnTokenReceivedInput,
  OnTokenReceivedOutput,
} from 'aws-amplify/push-notifications';
import {setDeviceToken} from './src/redux/deviceToken';
import {useDispatch} from 'react-redux';

export const RegisterToken = () : React.JSX.Element => {
  const dispatch = useDispatch();

  useEffect(() => {
    const tokenReceivedHandler: OnTokenReceivedInput = token => {
      // Do something with the received token
      console.log(token, 'token');
      dispatch(setDeviceToken(token));
    };

    const tokenListener: OnTokenReceivedOutput =
      onTokenReceived(tokenReceivedHandler);

    return () => {
      tokenListener.remove();
    };
  }, []);
  // @ts-ignore
  return <></>;
};

then in App.tsx:

...
import {RegisterToken} from './RegisterToken';
...
  // @ts-ignore
  return (
    <>
      <Provider store={store}>
        <PersistGate loading={<Text>Loading...</Text>} persistor={persistor}>
          <QueryClientProvider client={queryClient}>
            <SafeAreaProvider>
              <ThemeProvider theme={theme}>
                <RegisterToken />
                <AppNavigation />
              </ThemeProvider>
            </SafeAreaProvider>
          </QueryClientProvider>
        </PersistGate>
      </Provider>
      <Toast />
    </>
  );
...

Is this approach supported?

cshfang commented 5 months ago

Hey @victoravr.

Yes, looks like it was the first network call to Pinpoint made by the app since launch.

Is there a network call made afterwards?

Where in the application are you calling initializePushNotifications and identifyUser? It seems like perhaps the identifyUser network call is happening between the endpoint has a chance to get the token registered. This ought not to happen and I'm hoping to better understand why it's happening in your application flow

cshfang commented 5 months ago

After testing this out, I do think we have a potential race condition if initializePushNotifications and identifyUser are called closely in sequence due to the fact that they are both trying to update a Pinpoint endpoint behind the scenes. There are a couple issues here:

  1. When creating an endpoint internally, both calls attempt to retrieve some cached endpoint - but if they are called back-to-back, the asynchronous nature of our cache lookup causes a potential for different endpoint IDs to be created. I am reopening this issue as a result of this and will track as a bug.
  2. Even with the above resolved, it appears possible for the network call associated to an identifyUser call to end up earlier in sequence than the one triggered by initializePushNotifications depending on your application. As a result, if an endpoint is created/updated with OptOut set to 'NONE' before the address is set by initializePushNotifications, you can run into the issue you're seeing above.

Issue 2 is a bit hairier to resolve as it is not clear to me why the Pinpoint service does not allow the use case of opting an endpoint out before attaching an address to it. So I am in contact with the Pinpoint team regarding this. Pending their response, I will work on evaluating possible solutions to this.

In the meantime, since you are listening for a token, I think a potential fix in the meantime could be to pass that token in along with your identifyUser call as the address.

victoravr commented 5 months ago

Hi @cshfang,

Is there a network call made afterwards?

Yes, looks like there was a call after that with address:

{
  "RequestId": "7d62b64a-bcca-48cd-872f-db17b3bcdca3",
  "EffectiveDate": "2024-04-28T01:04:49.043Z",
  "ChannelType": "APNS_SANDBOX",
  "Address": "80042cfb1a57d0426ad71ebe50...663931ca806",
  "Demographic": {
    "AppVersion": "ios/17.4",
    "Make": null,
    "Model": null,
    "ModelVersion": "17.4",
    "Platform": "ios"
  },
  "Location": {},
  "User": {
    "UserId": "ap-southeast-2:7de8bcb7-b604-cd23-5116-5211a83a494f"
  }
}

Where in the application are you calling initializePushNotifications and identifyUser? It seems like perhaps the identifyUser network call is happening between the endpoint has a chance to get the token registered. This ought not to happen and I'm hoping to better understand why it's happening in your application flow

I'm calling initializePushNotifications(); in root index.js after Amplify.configure(...); but before AppRegistry.registerComponent(appName, () => App);

As for identifyUser, it's inside useEffect in AppNavigation.js:

...
  useEffect(() => {
    if (user) {
      if (user.userId) {
        identifyUsertoPinpoint({
          deviceToken: deviceToken,
          userId: user.userId,
          optOut: 'NONE'
        });
      };
    };
  }, [user, deviceToken]);
...
cshfang commented 5 months ago 
...
  useEffect(() => {
    if (user) {
      if (user.userId) {
        identifyUsertoPinpoint({
          deviceToken: deviceToken,
          userId: user.userId,
          optOut: 'NONE'
        });
      };
    };
  }, [user, deviceToken]);
...

I think the network call happening out of order is consistent with my findings. However, I noticed that you are already sending the device token along with your identifyUser call. I don't know what the exact body of your identifyUsertoPinpoint function looks like - but two things we want to make sure of are:

  1. That deviceToken is passed along with the identifyUser call e.g. options: { address: deviceToken, optOut }
  2. That the address is never explicitly an empty string (this might cause weirdness with the service API)
victoravr commented 4 months ago

... make sure of are:

  1. That deviceToken is passed along with the identifyUser call e.g. options: { address: deviceToken, optOut }
  2. That the address is never explicitly an empty string (this might cause weirdness with the service API)

Thanks! Updated my code to only do API request with OptOut if deviceToken is not empty/null.

Question: when device token is sent to the end user device/simulator for APNs (e.g. in a Sandbox)?

At least on simulator, I've noticed it does not appear at all sometimes...

cshfang commented 4 months ago

Question: when device token is sent to the end user device/simulator for APNs (e.g. in a Sandbox)?

At least on simulator, I've noticed it does not appear at all sometimes...

It should be invoked when didRegisterForRemoteNotificationsWithDeviceToken is invoked in the AppDelegate - so essentially when an app is launched for the first time. On simulator the token is only reliably passed to the JS side after terminating the app, relaunching, and then doing an additional background + resume. It seems to be another quirk with the simulator at this time though.

cshfang commented 4 months ago

Hi @victoravr

Amplify JS v6.3.1 has just been released. It should contain the following fixes: https://github.com/aws-amplify/amplify-js/pull/13319 https://github.com/aws-amplify/amplify-js/pull/13353

We believe these fixes should resolve your issue of the Missing Address error being triggered by network race conditions which might result in an identifyUser call attempting to create an endpoint with OptOut: 'NONE' and no Address. Could you please help us by verifying if this is indeed the case by updating to the latest Amplify version? Thank you

victoravr commented 4 months ago

... Could you please help us by verifying if this is indeed the case by updating to the latest Amplify version?

Hi @cshfang ,

I've installed Amplify JS v6.3.1, and tested my app on a physical iPhone device.

Unfortunately, notifications did not work initially due to a token not being issued after launching the app.

I had to close the app, re-launch it, sign-in as AWS Cognito user, then minimize it and re-open - and only then APNs token had been issued... not sure why this is the case..

Any advise? Many thanks!

cshfang commented 4 months ago

Hmm there should definitely be no reason for a need to sign in as a user as the endpoints are tied to device. Was the token not issued from a fresh install? We are responding to the didRegisterForRemoteNotificationsWithDeviceToken callback from the native side so it's not quite clear why this might be happening.

Could you try to add NSLog to your AppDelegate to make sure that didRegisterForRemoteNotificationsWithDeviceToken is being called but not propagating to the React Native side?

cshfang commented 4 months ago

@victoravr Thanks for your patient on this stuff, by the way. Would you mind opening a separate ticket to track the token issue? Given that I have seen similar behavior on simulator with iOS 17, I am wondering if something changed in the bootstrapping behavior of the observers which fire the registration call in the native layer. It would be helpful if we could track the investigation of this on a ticket dedicated to that.