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

Push Notification Configuration fails (on Android only) when multiple API endpoints are present #5423

Open
2 of 14 tasks
bitsplash-andy opened this issue Sep 5, 2024 · 7 comments
Labels
core Issues related to the Amplify Core Plugin question A question about the Amplify Flutter libraries

Comments

@bitsplash-andy
Copy link

bitsplash-andy commented Sep 5, 2024

Description

I am unable to configure push notifications on Android using version 2.4.1 of all plugins when having two GraphQL APIs defined in the config. All functionality works as expected on iOS.

It looks as though the push notifications code has been updated to use Gen2 Amplify Outputs via: #5073

There is a conditional platform statement on line 233 which encodes the config for storage in the SecureStorage.

This causes the following exception to be thrown

Converting object to an encodable object failed: Instance of 'AmplifyOutputs'
#0      _JsonStringifier.writeObject (dart:convert/json.dart:793:7)
#1      _JsonStringStringifier.printOn (dart:convert/json.dart:982:17)
#2      _JsonStringStringifier.stringify (dart:convert/json.dart:967:5)
#3      JsonEncoder.convert (dart:convert/json.dart:345:30)
#4      JsonCodec.encode (dart:convert/json.dart:231:45)
#5      jsonEncode (dart:convert/json.dart:114:10)
#6      AmplifyPushNotifications.configure (package:amplify_push_notifications/src/amplify_push_notifications_impl.dart:238:16)

The cause of this is:

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/amplify_core/lib/src/config/amplify_outputs/amplify_outputs.dart#L111C1-L125C2

As I say, this breaks Push Notifications on Android only when the API config contains more than one endpoint. All other categories function as expected if the Push Notification plugin is removed, and there are no issues on iOS even when plugin is present.

I'm assuming more things may break further down the line as the Gen2 code is adopted into more packages, but it's frustrating to have such an edge case appear after migrating the codebase to the latest version.

Categories

  • Analytics
  • API (REST)
  • API (GraphQL)
  • Auth
  • Authenticator
  • DataStore
  • Notifications (Push)
  • Storage

Steps to Reproduce

Add more then one API endpoint to the configuration

add AmplifyPushNotificationsPinpoint() via Amplify.addPlugins
call Amplify.configure

Screenshots

No response

Platforms

  • iOS
  • Android
  • Web
  • macOS
  • Windows
  • Linux

Flutter Version

3.24.1

Amplify Flutter Version

2.4.1

Deployment Method

Custom Pipeline

Schema

No response

@github-actions github-actions bot added pending-triage This issue is in the backlog of issues to triage pending-maintainer-response Pending response from a maintainer of this repository labels Sep 5, 2024
@bitsplash-andy
Copy link
Author

bitsplash-andy commented Sep 5, 2024

Rolling back to version 2.1.0 prevents issue, but is far from ideal.

Need to understand why AmplifyOutputs contains the following:

/// Converts a map of [DataOutputs] to a single data json object.
///
/// This manual mapping is required since the Amplify Outputs schema only supports
/// a single data object, but Amplify Flutter supports more than 1.
Object? _dataToJson(Map<String, DataOutputs>? outputs) {
  if (outputs == null) return null;
  if (outputs.length > 1) {
    throw ConfigurationError(
      'Found ${outputs.length} endpoints.'
      ' Amplify Outputs does not support multiple GraphQL endpoints.',
    );
  }
  final data = outputs.values.firstOrNull;
  return data?.toJson();
}

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/amplify_core/lib/src/config/amplify_outputs/amplify_outputs.dart#L111C1-L125C2

and

/// Converts a single data json object to a map of [DataOutputs].
///
/// This manual mapping is required since the Amplify Outputs schema only supports
/// a single data object, but Amplify Flutter supports more than 1.
Map<String, DataOutputs>? _dataFromJson(Map<String, Object?>? object) {
  if (object == null) return null;
  return {
    _dataPluginName: DataOutputs.fromJson(object),
  };
}

@bitsplash-andy
Copy link
Author

bitsplash-andy commented Sep 5, 2024

Failing a change to AmplifyOutputs, perhaps the following should be modified to use a custom JSON encoder to prevent the incompatibility for now:

await _amplifySecureStorage.write(
  key: configSecureStorageKey,
  value: jsonEncode(config),
);

and

final amplifyconfigStr = await amplifySecureStorage.read(
  key: configSecureStorageKey,
);

@bitsplash-andy bitsplash-andy changed the title Push Notification Configuration on Android fails when multiple API endpoints are present Push Notification Configuration fails (on Android only) when multiple API endpoints are present Sep 5, 2024
@NikaHsn
Copy link
Member

NikaHsn commented Sep 5, 2024

Sorry that you are facing this issue and thanks for reporting it. We will look into this and get back to you when we have updates.

@github-actions github-actions bot removed the pending-maintainer-response Pending response from a maintainer of this repository label Sep 5, 2024
@NikaHsn
Copy link
Member

NikaHsn commented Sep 6, 2024

@bitsplash-andy The Amplify CLI (both Generations 1 and 2) does not support multiple GraphQL API endpoints. Hence, the Amplify Flutter libraries are designed to handle configurations with only one GraphQL API endpoint. While the AmplifyOutputs type allows for multiple data endpoints, this is primarily for internal implementations and backwards compatibility. When using custom pipeline for deployment please make sure the amplify configuration is a valid josn for this schema.

@NikaHsn NikaHsn added question A question about the Amplify Flutter libraries core Issues related to the Amplify Core Plugin labels Sep 6, 2024
@github-actions github-actions bot removed the pending-triage This issue is in the backlog of issues to triage label Sep 6, 2024
@NikaHsn NikaHsn added the pending-community-response Pending response from the issue opener or other community members label Sep 6, 2024
@bitsplash-andy
Copy link
Author

bitsplash-andy commented Sep 6, 2024

Hi @NikaHsn, thanks for your reply.

To clarify, we don’t use the Amplify CLI, everything is provisioned manually via CDK etc. We only use the client library.

The client library has supported multiple API endpoints without issue up until version 2.2.0, at which point this began failing in the push notification library as described above.

for example the following :

"api": {
      "plugins": {
          "awsAPIPlugin": {
              "Authenticated": {
                  "endpointType": "GraphQL",
                  "endpoint": "ENDPOINT_URL",
                  "region": "eu-west-2",
                  "authorizationType": "AMAZON_COGNITO_USER_POOLS"
              },
              "Unauthenticated": {
                  "endpointType": "GraphQL",
                  "endpoint": "ENDPOINT_URL",
                  "region": "eu-west-2",
                  "authorizationType": "API_KEY",
                  "apiKey": "KEY"
              }
          }
      }
    },

can be switched at runtime via

  final request = GraphQLRequest(
    apiName: 'Authenticated',
    document: '''query someQuery {}''',
  );

or

  final request = GraphQLRequest(
    apiName: 'Unauthenticated',
    document: '''query someQuery {}''',
  );

Docs here

Multiple API endpoint calls continue to work as expected if the push notifications plugin is disabled, but the enforced single endpoint schema on configuration serialisation described above causes the push notification plugin to throw an exception on initialisation if multiple API endpoints are present. As the configuration cannot be specified on a plugin basis, this means that the choice appears to be stay on version 2.1.0 or loose the ability to enable the push notifications plugin in newer releases.

Additionally, this only happens on Android. From a library consumption perspective for users that want to use existing, non-Amplify provisioned backends this is a real issue.

@github-actions github-actions bot added pending-maintainer-response Pending response from a maintainer of this repository and removed pending-community-response Pending response from the issue opener or other community members labels Sep 6, 2024
@bitsplash-andy
Copy link
Author

Apologies, I've updated my previous comment a few times for clarity.

@NikaHsn
Copy link
Member

NikaHsn commented Sep 6, 2024

thanks @bitsplash-andy for clarification. we will look into this and get back to you when we have update.

@github-actions github-actions bot removed the pending-maintainer-response Pending response from a maintainer of this repository label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues related to the Amplify Core Plugin question A question about the Amplify Flutter libraries
Projects
None yet
Development

No branches or pull requests

4 participants
@NikaHsn @bitsplash-andy and others