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

Sequential update on a datastore model drops changes from all except the first one #11674

Open itsramiel opened 1 year ago

itsramiel commented 1 year ago

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

DataStore

Amplify Categories

api

Environment information

``` # Put output below this line System: OS: macOS 13.4.1 CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Memory: 2.06 GB / 16.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node Yarn: 1.22.19 - ~/.nvm/versions/node/v16.16.0/bin/yarn npm: 9.7.2 - ~/.nvm/versions/node/v16.16.0/bin/npm pnpm: 7.2.1 - ~/.nvm/versions/node/v16.16.0/bin/pnpm Watchman: 2023.04.17.00 - /usr/local/bin/watchman Browsers: Chrome: 114.0.5735.198 Safari: 16.5.2 npmPackages: @babel/core: ^7.20.0 => 7.22.9 @react-native-async-storage/async-storage: 1.17.11 => 1.17.11 @react-native-community/netinfo: 9.3.7 => 9.3.7 @types/react: ~18.0.14 => 18.0.38 HelloWorld: 0.0.1 amazon-cognito-identity-js: ^6.3.1 => 6.3.1 amazon-cognito-identity-js/internals: undefined () aws-amplify: ^5.3.4 => 5.3.4 core-js: ^3.31.1 => 3.31.1 expo: ~48.0.18 => 48.0.20 expo-status-bar: ~1.4.4 => 1.4.4 react: 18.2.0 => 18.2.0 react-native: 0.71.8 => 0.71.8 typescript: ^4.9.4 => 4.9.5 (5.0.2) npmGlobalPackages: @aws-amplify/cli: 11.1.1 @fsouza/prettierd: 0.23.2 cordova: 11.0.0 corepack: 0.11.2 eas-cli: 3.15.1 eslint_d: 12.2.1 expo-cli: 6.3.10 ios-deploy: 1.11.4 npm: 9.7.2 react-devtools: 4.24.7 typescript-language-server: 3.3.0 typescript: 4.9.4 vscode-langservers-extracted: 4.6.0 yarn: 1.22.19 ```

Describe the bug

I have a model that I update by querying, copying and then saving it:

const updateTodo = async (name: string) => {
    const todo = (await DataStore.query(Todo))[0];

    if (todo) {
      await DataStore.save(
        Todo.copyOf(todo, (draft) => {
          draft.name = name;
        })
      );
    }
  };

When I try to update the model multiple times sequentially like this:

          await updateTodo("1");
          await updateTodo("2");
          await updateTodo("3");
          await updateTodo("4");

Observing the model and logging the name shows:

image

The updates after the first one are discarded.

Expected behavior

I should not lose update to a model, and the latest update should be reflected

Reproduction steps

  1. Follow the amplify react native tutorial to set up a project. Paste the following in your App.tsx:
    
    import "core-js/full/symbol/async-iterator";

import { Amplify, DataStore } from "aws-amplify"; import awsExports from "./src/aws-exports"; import { Todo } from "./src/models"; import { useEffect } from "react"; import { Button, View } from "react-native"; Amplify.configure(awsExports);

const App = () => { useEffect(() => { const subscription = DataStore.observeQuery(Todo).subscribe( ({ items, isSynced }) => { console.log(items[0]?.name); } );

return () => {
  subscription.unsubscribe();
};

}, []);

const updateTodo = async (name: string) => { const todo = (await DataStore.query(Todo))[0];

if (todo) {
  await DataStore.save(
    Todo.copyOf(todo, (draft) => {
      draft.name = name;
    })
  );
}

};

return ( <View style={{ flex: 1, justifyContent: "center", alignItems: "center" }}> <Button title="update" onPress={async () => { await updateTodo("1"); await updateTodo("2"); await updateTodo("3"); await updateTodo("4"); }} /> ); };

export default App;


### Code Snippet

```javascript
// Put your code below this line.

Log output

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

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

chrisbonifacio commented 1 year ago

Hi @itsramiel , this seems to be due to a race condition where the first save sends the mutation to update the record in the DB and when that succeeds the record version will have incremented. This happens before the rest of the requests are sent out, which means they are sent with a lower version than is now on the server, resulting in the updates being ignored. This can be confirmed by checking the network activity and the payloads sent to appsync for each mutation.

One workaround might be to add a short delay between the saves, or wait for the outboxMutationProcessed event before performing the next save.

itsramiel commented 1 year ago

Hey @chrisbonifacio Thank you for the reply. Yes I am aware of why it happens. Adding a short delay between saves could save it but it depends on the internet speed. Listening for the outboxMutationProcessed event would require to handle offline state.

Also it doesnt make sense for all amplify users to have their own workarounds. It is better if it was baked into it. Is this something the team is aware of and looking to fix?

chrisbonifacio commented 1 year ago

Hi @itsramiel I've marked this as a feature request so we can look into a more holistic solution. Thank you 🙏

kevinlam92 commented 4 weeks ago

@chrisbonifacio

I've been digging deeper into this issue and read through some PRs such as this: https://github.com/aws-amplify/amplify-js/pull/7354

At a surface level this should be fixed by the PR mentioned above. But it still seems like the "race condition" explanation is still pretty rampant every time someone posts this issue.

My question is, where is the supposed race condition exactly? If it is the case of inflight requests wouldn't @iartemiev 's changes in that PR fix this? At what point does DataStore decide to merge models in the outbox? Why do we have to wait for outbox mutation to be processed before attempting to save (won't it queued in outbox and merged automatically when the response comes back)?

In our project we've created a shim on top of DataStore save to be aware of outbox mutations based on model id, such that saves are enqueued and merged if there is an outbox mutation in process. But this begs the question, what's the point of the outbox merger if that's what I have to do?