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

When a user not signed in mutation events are removed. #10013

Closed capitaldotcodes closed 1 year ago

capitaldotcodes commented 2 years ago

Before opening, please confirm:

JavaScript Framework

Angular

Amplify APIs

DataStore

Amplify Categories

Not applicable

Environment information

``` System: OS: macOS 12.4 CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz Memory: 89.74 MB / 32.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node npm: 8.1.2 - ~/.nvm/versions/node/v16.13.1/bin/npm Browsers: Chrome: 103.0.5060.53 Firefox: 81.0.1 Safari: 15.5 npmPackages: @angular-builders/custom-webpack: ^12 => 12.1.3 @angular-devkit/build-angular: ~12 => 12.2.17 @angular/animations: ~12 => 12.2.16 @angular/animations/browser: undefined () @angular/animations/browser/testing: undefined () @angular/cli: ~12 => 12.2.17 @angular/common: ~12 => 12.2.16 @angular/common/http: undefined () @angular/common/http/testing: undefined () @angular/common/testing: undefined () @angular/common/upgrade: undefined () @angular/compiler: ~12 => 12.2.16 (9.0.0) @angular/compiler-cli: ~12 => 12.2.16 @angular/compiler/testing: undefined () @angular/core: ~12 => 12.2.16 (9.0.0) @angular/core/testing: undefined () @angular/forms: ~12 => 12.2.16 @angular/language-service: ~12 => 12.2.16 @angular/platform-browser: ~12 => 12.2.16 @angular/platform-browser-dynamic: ~12 => 12.2.16 @angular/platform-browser-dynamic/testing: undefined () @angular/platform-browser/animations: undefined () @angular/platform-browser/testing: undefined () @angular/router: ~12 => 12.2.16 @angular/router/testing: undefined () @angular/router/upgrade: undefined () @auth0/auth0-angular: ^1.10.0 => 1.10.0 @babel/cli: ^7.12.1 => 7.17.10 @babel/core: ^7.12.3 => 7.18.2 (7.14.8) @babel/preset-env: ^7.12.1 => 7.18.2 (7.14.8) @babel/preset-typescript: ^7.12.1 => 7.17.12 @capacitor/android: ^3.5.1 => 3.5.1 @capacitor/app: 1.1.1 => 1.1.1 @capacitor/browser: ^1.0.7 => 1.0.7 @capacitor/cli: 3.5.1 => 3.5.1 @capacitor/core: 3.5.1 => 3.5.1 @capacitor/device: ^1.1.2 => 1.1.2 @capacitor/filesystem: ^1.1.0 => 1.1.0 @capacitor/geolocation: ^1.3.1 => 1.3.1 @capacitor/haptics: 1.1.4 => 1.1.4 @capacitor/ios: ^3.5.1 => 3.5.1 @capacitor/keyboard: 1.2.2 => 1.2.2 @capacitor/local-notifications: ^1.1.0 => 1.1.0 @capacitor/share: ^1.1.2 => 1.1.2 @capacitor/splash-screen: ^1.2.2 => 1.2.2 @capacitor/status-bar: ^1.0.8 => 1.0.8 @ionic-native/app-version: 5.36.0 => 5.36.0 @ionic-native/background-mode: 5.36.0 => 5.36.0 @ionic-native/core: ^5.36.0 => 5.36.0 @ionic-native/native-audio: 5.36.0 => 5.36.0 @ionic/angular: ^6.1.7 => 6.1.7 @ionic/angular-toolkit: ^6.1.0 => 6.1.0 @ionic/storage: ~3.0.6 => 3.0.6 @mapbox/mapbox-gl-draw: ^1.2.0 => 1.3.0 @mapbox/mapbox-gl-style-spec: 13.20.1 @mapbox/mapbox-sdk: ^0.11.0 => 0.11.0 @mapbox/togeojson: ~0.16.0 => 0.16.0 @mapbox/whoots-js: ~3.1.0 => 3.1.0 @ngrx/effects: ^12 => 12.5.1 @ngrx/effects/testing: undefined () @ngrx/entity: ^12 => 12.5.1 @ngrx/schematics: ^12.0.0 => 12.5.1 @ngrx/store: ^12 => 12.5.1 @ngrx/store-devtools: ^12.0.0 => 12.5.1 @ngrx/store/testing: undefined () @turf/turf: 5.1.6 => 5.1.6 @types/geojson: 7946.0.7 => 7946.0.7 @types/jasmine: ~3.5.0 => 3.5.14 @types/jasminewd2: ~2.0.3 => 2.0.10 @types/lodash: 4.14.141 => 4.14.141 @types/mapbox: ^1.6.41 => 1.6.42 @types/mapbox-gl: 2.3.0 => 2.3.0 @types/mapbox__mapbox-sdk: ^0.6.5 => 0.6.5 @types/moment-timezone: ~0.5.10 => 0.5.30 @types/node: ^12.11.1 => 12.20.52 @xstate/fsm: ^1.5.1 => 1.6.5 almost-equal: 1.1.0 => 1.1.0 auth0-js: ^9.19.0 => 9.19.0 aws-amplify: ^4.3.26 => 4.3.26 codelyzer: ^6.0.0 => 6.0.2 cordova-plugin-app-version: 0.1.9 => 0.1.9 cordova-plugin-background-mode: 0.7.3 => 0.7.3 cordova-plugin-device: ^2.1.0 => 2.1.0 cordova-plugin-device-electron: 1.0.0 cordova-plugin-nativeaudio: 3.0.9 => 3.0.9 core-js: 3.2.1 => 3.2.1 (3.16.0, 3.22.8) crypto-browserify: ^3.12.0 => 3.12.0 es6-promise-plugin: ^4.2.2 => 4.2.2 example-typescript: 1.0.0 geoposition-to-object: 1.0.2 => 1.0.2 jasmine-spec-reporter: ~5.0.0 => 5.0.2 karma: ~6.3.20 => 6.3.20 karma-chrome-launcher: ~3.1.0 => 3.1.1 karma-coverage-istanbul-reporter: ~3.0.2 => 3.0.3 karma-jasmine: ~3.3.0 => 3.3.1 karma-jasmine-html-reporter: ^1.5.0 => 1.7.0 lib: 0.0.1 list: 2.0.19 => 2.0.19 list/curried: undefined () list/fantasy-land: undefined () list/methods: undefined () list/ramda: undefined () lodash: 4.17.15 => 4.17.15 (4.17.21) mapbox: ^1.0.0-beta10 => 1.0.0-beta10 mapbox-gl: 2.3.1 => 2.3.1 moment: ~2.24.0 => 2.24.0 moment-timezone: ~0.5.23 => 0.5.34 ngx-mapbox-gl: ^7.1.2 => 7.1.2 node-example: 1.0.0 protractor: ~7.0.0 => 7.0.0 protractor-example: 1.0.0 rxjs: ~6 => 6.6.7 (7.5.5) rxjs/ajax: undefined () rxjs/fetch: undefined () rxjs/internal-compatibility: undefined () rxjs/operators: undefined () rxjs/testing: undefined () rxjs/webSocket: undefined () serialize-error: ~5.0.0 => 5.0.0 (2.1.0) stream-browserify: ^3.0.0 => 3.0.0 tokml: ^0.4.0 => 0.4.0 ts-node: ~8.3.0 => 8.3.0 (10.8.0) tslib: ^2.0.0 => 2.4.0 (2.3.0, 1.14.1) tslint: ~6.1.0 => 6.1.3 typescript: ~4.2.3 => 4.2.4 (4.6.4, 4.3.5) typescript-example: 1.0.0 uuid: ^8.3.1 => 8.3.2 (3.4.0, 3.3.2, 7.0.3) zone-mix: undefined () zone-node: undefined () zone-testing: undefined () zone.js: ~0.11.4 => 0.11.5 (0.10.3) zone.js/async-test: undefined () zone.js/async-test.min: undefined () zone.js/fake-async-test: undefined () zone.js/fake-async-test.min: undefined () zone.js/jasmine-patch: undefined () zone.js/jasmine-patch.min: undefined () zone.js/long-stack-trace-zone: undefined () zone.js/long-stack-trace-zone.min: undefined () zone.js/mocha-patch: undefined () zone.js/mocha-patch.min: undefined () zone.js/proxy: undefined () zone.js/proxy.min: undefined () zone.js/sync-test: undefined () zone.js/sync-test.min: undefined () zone.js/task-tracking: undefined () zone.js/task-tracking.min: undefined () zone.js/webapis-media-query: undefined () zone.js/webapis-media-query.min: undefined () zone.js/webapis-notification: undefined () zone.js/webapis-notification.min: undefined () zone.js/webapis-rtc-peer-connection: undefined () zone.js/webapis-rtc-peer-connection.min: undefined () zone.js/webapis-shadydom: undefined () zone.js/webapis-shadydom.min: undefined () zone.js/wtf: undefined () zone.js/wtf.min: undefined () zone.js/zone-bluebird: undefined () zone.js/zone-bluebird.min: undefined () zone.js/zone-error: undefined () zone.js/zone-error.min: undefined () zone.js/zone-legacy: undefined () zone.js/zone-legacy.min: undefined () zone.js/zone-patch-canvas: undefined () zone.js/zone-patch-canvas.min: undefined () zone.js/zone-patch-cordova: undefined () zone.js/zone-patch-cordova.min: undefined () zone.js/zone-patch-electron: undefined () zone.js/zone-patch-electron.min: undefined () zone.js/zone-patch-fetch: undefined () zone.js/zone-patch-fetch.min: undefined () zone.js/zone-patch-jsonp: undefined () zone.js/zone-patch-jsonp.min: undefined () zone.js/zone-patch-message-port: undefined () zone.js/zone-patch-message-port.min: undefined () zone.js/zone-patch-promise-test: undefined () zone.js/zone-patch-promise-test.min: undefined () zone.js/zone-patch-resize-observer: undefined () zone.js/zone-patch-resize-observer.min: undefined () zone.js/zone-patch-rxjs: undefined () zone.js/zone-patch-rxjs-fake-async: undefined () zone.js/zone-patch-rxjs-fake-async.min: undefined () zone.js/zone-patch-rxjs.min: undefined () zone.js/zone-patch-socket-io: undefined () zone.js/zone-patch-socket-io.min: undefined () zone.js/zone-patch-user-media: undefined () zone.js/zone-patch-user-media.min: undefined () npmGlobalPackages: @angular/cli: 13.3.7 @aws-amplify/cli: 8.3.1 @ionic/cli: 6.19.1 cordova-res: 0.15.4 cordova: 11.0.0 corepack: 0.10.0 ios-deploy: 1.11.4 native-run: 1.5.0 npm: 8.1.2 ```

Describe the bug

When a user is signed out any model which is created via Datastore.save() persists but after erroring and retrying the mutation is deleted from the queue.

One the mutation event is deleted the model is never synced and remains locally available but not remotely available.

The responsible line of code is below. Commenting out this dequeue operation appears to fix the problem. What the consequences are of doing this I am unaware.

https://github.com/aws-amplify/amplify-js/blob/7656bc8a9864af05618d3fd794a48217e9ec7295/packages/datastore/src/sync/processors/mutation.ts#L207

Expected behavior

When a user is not logged in and creates models mutation events should not be deleted and on user signup i.e Datastore.stop Datastore.start the mutations should be processed by the sync engine.

I can see why this might be useful in some use cases but the developer has control of the local Datastore via Datastore.clear and thus the default behaviour should be that the mutations remain in the queue.

Reproduction steps

1.) Without a signed in user create a model 2.) In Application on chrome browser view sync_MutationEvent and note that mutation event has been deleted 3.) Create user and login 4.) Query for created model and see that it is available but has not been synced remotely

Code Snippet

// 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 2 years ago

Hi @capitaldotcodes 👋 thanks for raising this issue. Could you please share your schema to see what auth rules you might be using?

Is the model authorized to be created by public access (API_KEY) or by private access (Cognito User Pools, OIDC, IAM, etc)?

Or are you seeing this behavior with any model regardless of the auth rules set?

capitaldotcodes commented 2 years ago

I'm using Cognito User Pools i've tested in a very skinny example app and can reproduce the behaviour.

  @model
  @auth(
    rules: [
     { allow: owner }
    ]
  ) 
  {
  id: ID!
  name: String
  owner: String
}

When the user is signed out the mutation event is removed you can tell it is created as the local storage index increases.

dpilch commented 2 years ago

Possibly related to #6534.

Could further explain your use case for creating models when no user is signed in?

Or is it more the following case?

  1. Sign in user
  2. Create a new model
  3. Sign out user
  4. New model mutation is removed from queue.
capitaldotcodes commented 2 years ago

The app is offline first and does not require an account to use. When the user creates an account the models created locally should automatically sync. Given that when the mutation events are left in the queue amplify handles this seamlessly it would seem strange to imply the use case was not supported. This was how the datastore worked in all the AWS provided demo apps.

I don't think it is related to #6534. To me it is a problem with what is and is not a retry-able error. The no current user error should be a retry-able error and the mutation event shouldn't be deleted. Alas at the moment it retries, fails, gives up and deletes the event after logging 'done retrying'.

No the method to reproduce is 1.) Without a signed in user create a model 2.) In Application on chrome browser view sync_MutationEvent and note that mutation event has been deleted 3.) Create user and login 4.) Query for created model and see that it is available but has not been synced remotely

dpilch commented 2 years ago

Ok, I understand now. I agree it is strange behavior that the mutations are deleted. I'll look into some solutions for this.

david-mcafee commented 1 year ago

@capitaldotcodes - my apologies if I'm not understanding the reproduction steps correctly, but from what I see you're attempting to create a model when a user is not signed in, but your schema does not allow for public CRUD access - is that correct? If so, you would need to update your schema to allow public access, or only perform the mutation when the user is signed in. If I'm not understanding correctly, please let me know. Thank you!

david-mcafee commented 1 year ago

@capitaldotcodes - closing this ticket as we have not heard back from you. If you need further assistance, please let us know!