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.44k stars 2.13k forks source link

Have DataStore detect when GraphQL Schema has changed remotely in AppSync #9901

Open danrivett opened 2 years ago

danrivett commented 2 years ago

Is this related to a new or existing framework?

React Native

Is this related to a new or existing API?

DataStore

Is this related to another service?

No response

Describe the feature you'd like to request

We have an Expo app that we use DataStore with to interact with an AppSync backend. In order to do that we use Amplify codegen to build model types against a given version of a GraphQL schema deployed to AppSync via Amplify CLI.

Although DataStore seems to be able to detect when the local database was created/synced against a version different to the local schema now in use, DataStore doesn't appear to have a mechanism of detecting when the local schema in use differs from the remote AppSync schema.

This is quite important functionality for us, since we want to introduce a breaking change to our schema to refactor a model type so that we can add a GSI in DynamoDB but we have existing Expo applications installed on client devices.

We plan on releasing a new build of our app which has an updated local schema that is compatible, and we are also going to migrate the existing data on the backend. However we also want to make sure that existing app installations automatically update to be compatible.

Expo provides great functionality to support this, and we check when the app is first loaded and automatically download a newer version. However, for existing users where the app is backgrounded, or even in use when the remote AppSync GraphQL schema is updated, there does not appear to be a very good solution.

We can check Expo when an app is foregrounded, but that doesn't help users who happen to using the app during this change. We could periodically poll for Expo app updates, but that seems quite chatty for an edge case that we hope to happen very rarely.

There do not appear to be very good options currently. We're currently investigating implementing a monitor for the warning messages emitted by DataStore when a subscription fails:

Here's one when the subscription fails due to a field no longer existing (as was moved to a top-level field for GSI purposes):

[WARN] 11:18.748 DataStore - subscriptionError, Array [
  "Connection failed: {\"errors\":[{\"message\":\"Validation error of type FieldUndefined: Field 'previousField' in type 'Timesheet' is undefined @ 'onCreateTimesheet/nested/previousField'\"}]}",
]

But new fields added to the schema wouldn't cause an error, the app just wouldn't receive that data.

We're also investigating monitoring warn messages emitted by DataStore when a queued Mutation fails such as:

[WARN] 15:23.725 DataStore, Array [
  Object {
    "errorInfo": undefined,
    "errorType": undefined,
    "localModel": Timesheet {
      ...
    },
    "message": "Validation error of type FieldUndefined: Field 'previousField' in type 'Timesheet' is undefined @ 'createTimesheet/nested/previousField'",
    "operation": "Create",
    "remoteModel": null,
  },
]

Which occurs when a mutation attempts to write a field that no longer exists.

Or:

[WARN] 48:20.647 DataStore, Array [
  Object {
    "errorInfo": undefined,
    "errorType": undefined,
    "localModel": Timesheet {
      ...
    },
    "message": "Variable 'input' has coerced Null value for NonNull type 'String!'",
    "operation": "Create",
    "remoteModel": null,
  },
]

Which happens when a new required String field is missing on the mutation.

As you can see none of these scenarios - which are real life ones that don't necessary get surfaced in simpler POCs - have great solutions available for us (that I'm aware of).

Describe the solution you'd like

What I'd like is for DataStore to automatically detect when the remote GraphQL schema deployed to AppSync is different from the local one, and emit an event that the app can subscribe to and react to.

In this case the app could then check Expo for updates and display a message if one is found indicating the app requires updating and will reload. If an update is not found (this should not be the case, but could happen in offline situations) then the app will have to make a non-trivial decision about what the best option is here, but that's outside the scope of this discussion.

Either way, allowing the app using DataStore to recover in this situation is very important because GraphQL schemas will evolve over time, and it's not practical to think breaking changes will never be introduced, and even if the changes aren't breaking (say a new non-required field is added) it would still be very helpful for DataStore to detect a schema change and allow the app to update itself as needed so it can start populating that new optional field.

Describe alternatives you've considered

See initial outline about how we're currently considering monitoring the warning messages emited. This is of course a non-ideal and brittle option, and I'm very open to other alternatives.

(Incidentally the fact they are emitted as warning messages is questionable to me, since they are errors from my perspective, and really the app should get events that they can subscribe to, so they can properly react).

Additional context

I understand this likely requires an enhancement to AppSync for it to be able to communicate the schema version deployed, as I don't think there is a way of doing that from an AppSync client's perspective?

But I'm writing this up here from the perspective of an Expo app using DataStore, and I'm happy to create a follow-up ticket with AppSync as needed.

Is this something that you'd be interested in working on?

undefobj commented 2 years ago

Thanks much Dan. As noted in our other conversation this is something that we've been looking at recently in a couple of ways. First, we have just spiked a way to detect when a schema has a breaking change by working with the team at The Guild and integrating GraphQL Inspector. Most likely this will surface in the CLI so that if you're doing an amplify push we can stop deployments if there is a breaking change & prompt you to abort or continue. This will also come with headless methods for the CI/CD flow control.

Past the build phase you are correct we do a topological sort on the model schema in order to create foreign key relationships, referential integrity, and update/delete sequencing during the sync process. We're currently in the phase of a project where we're adding deeper runtime error checks and emitting events to surface more debugging information. You can see some of that work HERE. During that analysis we talked about a few things:

  1. Can we detect "drift" between the local schema and the deployed API? This is most likely possible by using the method above on the local model schema and the introspection schema in AppSync.
  2. If we can do this, when should it be done? Current thinking is during the base sync which is every 24 hours by default but you can configure as a customer to be more frequent if necessary.
  3. If it is done and drift is detected, what should the behavior be?

The last question is the most interesting one. We could clearly emit logging messages as you note above, especially for non-breaking changes to allow DataStore to operate in a degraded mode. An app developer could then have customers go to the app store to download the latest version which has been built & compiled using a new schema. This is probably the safest method. We've also got a prototype of logging to CloudWatch Logs from an app (which we'll be launching in the coming months) which would allow you to monitor the client errors, GraphQL selection set usage, etc. Therefore you could combine these two techniques to safely make changes over time.

Another thing that we have looked at is versioning schema and letting clients specify the version at runtime. This would indeed take a change in AppSync, It's something that we're looking at in the future, but isn't in the plans currently this year. If this is a route you're looking for a request on the AppSync Community Repo is helpful to drive interest.

An alternative to the above is something that you hint at which is automatic reconciliation of drift to the clients. This would be to "download" models locally to the client once drift is detected, using something such as an S3 location. I'm unsure of the feasibility of this just yet, it's tricky from a safety standpoint on the clients which have compiled binaries, and might make serialization in Android/iOS tricky. I also am unsure how this impacts App Store guidelines of updating deployed code. We'll need to research this more.

I've flagged this thread internally to a few people so feel free to add more thoughts on your requirements, especially around MVP capabilities for us to iteratively look at these requests.

danrivett commented 2 years ago

Thanks Richard. You provided a lot of really good detail on various aspects to schema drift that I want to think through more before I formulate any initial thoughts. I will do that next week once I've had sufficient time to fully think through what you've written in detail.

Thanks again, I really appreciate it.