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

Datastore Sync Reachability uses Navigator.onLine resulting in data loss when incorrectly displaying online #9699

Closed jameswillis99 closed 2 years ago

jameswillis99 commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

DataStore

Amplify Categories

storage

Environment information

``` System: OS: macOS 11.6.4 CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz Memory: 918.55 MB / 16.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 14.16.0 - ~/.nvm/versions/node/v14.16.0/bin/node Yarn: 1.22.17 - ~/.nvm/versions/node/v14.16.0/bin/yarn npm: 6.14.11 - ~/.nvm/versions/node/v14.16.0/bin/npm Browsers: Chrome: 99.0.4844.51 Edge: 99.0.1150.39 Safari: 15.3 npmPackages: @babel/cli: 7.17.0 => 7.17.0 @babel/core: 7.17.2 => 7.17.2 (7.15.5, 7.17.5, 7.9.0, 7.12.9) @babel/preset-env: ^7.0.0 => 7.16.11 (7.3.4, 7.9.0) @babel/preset-react: ^7.0.0 => 7.16.7 (7.9.1) @types/blob-util: 1.3.3 @types/bluebird: 3.5.29 @types/chai: 4.2.7 @types/chai-jquery: 1.1.40 @types/jest: ^24.0.18 => 24.9.1 @types/jquery: 3.3.31 @types/lodash: 4.14.149 @types/minimatch: 3.0.3 @types/mocha: 5.2.7 @types/node: ^8.9.5 => 8.10.66 (17.0.21) @types/puppeteer: 1.3.0 => 1.3.0 @types/sinon: 7.5.1 @types/sinon-chai: 3.2.3 babel-loader: ^8.0.0 => 8.2.3 (8.1.0) bundlewatch: ^0.3.1 => 0.3.3 codecov: ^3.6.5 => 3.8.3 compression-webpack-plugin: ^1.1.3 => 1.1.12 cypress: ^3.2.0 => 3.8.3 husky: ^3.0.5 => 3.1.0 jest: ^24.x.x => 24.9.0 (23.6.0) jest-config: 24.8.0 => 24.8.0 (24.9.0, 23.6.0) json-loader: ^0.5.7 => 0.5.7 lerna: ^3.13.1 => 3.22.1 prettier: ^2.4.1 => 2.5.1 (1.19.1) pretty-quick: ^1.11.1 => 1.11.1 rimraf: ^2.6.2 => 2.7.1 (2.6.3, 3.0.2, 2.2.8) rollup: ^0.67.4 => 0.67.4 rollup-plugin-commonjs: ^9.2.0 => 9.3.4 rollup-plugin-json: ^3.1.0 => 3.1.0 rollup-plugin-node-resolve: ^4.0.0 => 4.2.4 rollup-plugin-sourcemaps: ^0.4.2 => 0.4.2 rollup-plugin-typescript: ^1.0.0 => 1.0.1 source-map-loader: ^0.2.1 => 0.2.4 ts-jest: ^24.x.x => 24.3.0 tslint: ^5.7.0 => 5.20.1 tslint-config-airbnb: ^5.8.0 => 5.11.2 typedoc: ^0.16.9 => 0.16.11 typescript: ~3.8.3 => 3.8.3 (3.9.5, 3.7.7) uglifyjs-webpack-plugin: ^0.4.6 => 0.4.6 uuid-validate: ^0.0.3 => 0.0.3 webpack: ^4.32.0 => 4.46.0 (4.42.0) webpack-bundle-analyzer: ^3.3.2 => 3.9.0 webpack-cli: ^3.1.0 => 3.3.12 winston: ^3.2.1 => 3.6.0 wml: 0.0.83 => 0.0.83 npmGlobalPackages: @aws-amplify/datastore: 3.7.8 aws-amplify-monorepo: 0.1.30 aws-amplify: 4.3.16 npm: 6.14.11 verdaccio: 5.7.1 yarn: 1.22.17 ```

Describe the bug

Datastore uses navigator.onLine which does a good job of detecting offline but is not reliable enough for detecting when online as it does not always check if it can reach the internet. This causes the app to believe its online (when actually offline) and timeout the jittered mutations before dequeuing the data and being lost from the sync process.

For example our use case is as follows:

A person using a windows go surface tablet (windows 10) will be in an area with very low signal/no signal for up to a whole day. They want to be able to sync their data up if there is signal, but often they can go hours without any internet connection. This issue causes the device sometimes believes it is online and will start attempting mutations which get jitteredretry'd and eventually fail and drop from the outbox. Our data is quite important and losing it is quite a big issue for us

Expected behavior

The app should properly detect if it can reach the endpoint, and not discard data if it has no network connection.

I have had to fork and modify source code locally as our project has a tight deadline, I fixed it by making an API call to the APPSYNC endpoint as well as if the sync mutation fails for network error it will turn app to offline and stop processing more mutations.

I'm happy to put up this PR and take advice to get this fixed. (possibly a websocket would be better than a setinterval which i've used to ping the backend - but unsure how to implement in this project)

Reproduction steps

  1. Amplify Datastore App.
  2. Sync offline data
  3. Unplug router from internet/ cause device to not have internet access but amplify navigator.onLine still believes it is online. On some windows devices sometimes disconnecting from WIFI can even cause this.
  4. Mutate some data
  5. Jittered retry will try multiple attempts at mutation which fail due to network connection issue and will eventually get dequeued from outbox after max retries.
  6. On establishing internet connection again the data will not get synced and other devices viewing data will not ever receive this data.

aws-exports.js

const awsmobile = { "aws_project_region": "eu-west-2", "aws_cognito_identity_pool_id": "eu-west-2:xxxxxxxxxxxxxxxxxxxxx", "aws_cognito_region": "eu-west-2", "aws_user_pools_id": "eu-west-xxxxxxxxxxxxxxxxxxxxx", "aws_user_pools_web_client_id": "xxxxxxxxxxxxxxxxxxxxx", "oauth": {}, "aws_cognito_username_attributes": [ "EMAIL" ], "aws_cognito_social_providers": [], "aws_cognito_signup_attributes": [], "aws_cognito_mfa_configuration": "OFF", "aws_cognito_mfa_types": [], "aws_cognito_password_protection_settings": { "passwordPolicyMinLength": 8, "passwordPolicyCharacters": [ "REQUIRES_LOWERCASE", "REQUIRES_UPPERCASE", "REQUIRES_NUMBERS", "REQUIRES_SYMBOLS" ] }, "aws_cognito_verification_mechanisms": [ "EMAIL" ], "aws_appsync_graphqlEndpoint": "xxxxxxxxxxxxxxxxxxxxx", "aws_appsync_region": "eu-west-2", "aws_appsync_authenticationType": "AMAZON_COGNITO_USER_POOLS", "aws_cloud_logic_custom": [ { "name": "xxxxxxxxxxxxxxxxxxxxx", "endpoint": "xxxxxxxxxxxxxxxxxxxxx", "region": "eu-west-2" } ], "aws_user_files_s3_bucket": "xxxxxxxxxxxxxxxxxxxxx", "aws_user_files_s3_bucket_region": "eu-west-2" };

module.exports = awsmobile;

laclance commented 2 years ago

perhaps related #9282

svidgen commented 2 years ago

This should be solved as of version 4.3.17 of aws-amplify. If you can, please give it a try and confirm!

jameswillis99 commented 2 years ago

In a sense yes it's been resolved, although possibly not all network errors are triggering the "Network Error" catch as we still have a couple of mutations which get isolated on the client.

Is there anything I can track in relation to the Reachability improvements?

jameswillis99 commented 2 years ago

Happy to close this colleague has raised another ticket https://github.com/aws-amplify/amplify-js/issues/9741

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.