aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
88 stars 76 forks source link

Hub.listen not receiving events from 'datastore' scope #891

Closed duranmla closed 1 year ago

duranmla commented 1 year ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

DataStore

Amplify Categories

No response

Environment information

``` System: OS: macOS 12.4 CPU: (8) arm64 Apple M1 Memory: 114.70 MB / 16.00 GB Shell: 5.8.1 - /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: 8.15.1 - ~/.nvm/versions/node/v16.16.0/bin/npm Browsers: Chrome: 105.0.5195.125 Firefox: 104.0.2 Safari: 15.5 npmPackages: @aws-crypto/sha256-js: ^2.0.1 => 2.0.1 (1.0.0-alpha.0, 1.2.2, 2.0.0) @commitlint/cli: ^17.0.3 => 17.0.3 @commitlint/config-conventional: ^17.0.3 => 17.0.3 @craco/craco: ^6.4.5 => 6.4.5 @cypress/mount-utils: 0.0.0-development @cypress/react: 0.0.0-development @cypress/react18: 0.0.0-development @cypress/vue: 0.0.0-development @cypress/vue2: 0.0.0-development @jackfranklin/test-data-bot: ^2.0.0 => 2.0.0 @react-theming/storybook-addon: ^1.1.7 => 1.1.7 @reduxjs/toolkit: ^1.8.3 => 1.8.3 @reduxjs/toolkit-query: 1.0.0 @reduxjs/toolkit-query-react: 1.0.0 @sentry/react: ^7.8.1 => 7.8.1 @sentry/tracing: ^7.8.1 => 7.8.1 @sentry/webpack-plugin: ^1.19.0 => 1.19.0 @storybook/addon-essentials: ^6.5.9 => 6.5.9 @storybook/addon-links: ^6.5.9 => 6.5.9 @storybook/addons: ^6.5.9 => 6.5.9 @storybook/builder-webpack5: ^6.5.9 => 6.5.9 @storybook/manager-webpack5: ^6.5.9 => 6.5.9 @storybook/preset-create-react-app: ^4.1.2 => 4.1.2 @storybook/react: ^6.5.9 => 6.5.9 @storybook/theming: ^6.5.9 => 6.5.9 @svgr/plugin-prettier: 5.5.0 => 6.3.1 @testing-library/cypress: ^8.0.3 => 8.0.3 @testing-library/dom: ^8.16.0 => 8.16.0 @testing-library/jest-dom: ^5.16.4 => 5.16.4 @testing-library/react: ^13.3.0 => 13.3.0 @testing-library/user-event: ^14.4.1 => 14.4.1 @types/draft-js: ^0.11.9 => 0.11.9 @types/draftjs-to-html: ^0.8.1 => 0.8.1 @types/html-to-draftjs: ^1.4.0 => 1.4.0 @types/jest: ^28.1.6 => 28.1.6 @types/lodash: ^4.14.182 => 4.14.182 @types/mixpanel-browser: ^2.38.0 => 2.38.0 @types/node: ^18.6.3 => 18.6.3 (16.11.46, 14.18.22) @types/react: ^18.0.15 => 18.0.15 @types/react-dom: ^18.0.6 => 18.0.6 @types/react-draft-wysiwyg: ^1.13.4 => 1.13.4 @types/react-html-parser: ^2.0.2 => 2.0.2 @types/react-redux: ^7.1.24 => 7.1.24 @types/react-router-dom: ^5.3.3 => 5.3.3 @types/react-slider: ^1.3.1 => 1.3.1 @types/redux-logger: ^3.0.9 => 3.0.9 @types/styled-components: ^5.1.25 => 5.1.25 @types/styled-system: ^5.1.15 => 5.1.15 @types/styled-system__css: ^5.0.17 => 5.0.17 @types/three: ^0.143.0 => 0.143.0 aws-amplify: ^4.3.30 => 4.3.30 aws-sdk: ^2.1187.0 => 2.1187.0 babel-plugin-styled-components: ^2.0.7 => 2.0.7 base64-js: ^1.5.1 => 1.5.1 cypress: ^10.4.0 => 10.4.0 dayjs: ^1.11.5 => 1.11.5 (1.11.4) draft-js: ^0.11.7 => 0.11.7 draftjs-to-html: ^0.9.1 => 0.9.1 eslint-config-prettier: ^8.5.0 => 8.5.0 eslint-plugin-cypress: ^2.12.1 => 2.12.1 eslint-plugin-prettier: ^4.2.1 => 4.2.1 formik: ^2.2.9 => 2.2.9 framer-motion: ^6.5.1 => 6.5.1 history: ^5.3.0 => 5.3.0 html-to-draftjs: ^1.5.0 => 1.5.0 husky: ^8.0.1 => 8.0.1 lodash: ^4.17.21 => 4.17.21 mixpanel-browser: ^2.45.0 => 2.45.0 polished: ^4.2.2 => 4.2.2 postcss-normalize: ^10.0.1 => 10.0.1 prettier: ^2.7.1 => 2.7.1 (2.3.0) pretty-quick: ^3.1.3 => 3.1.3 react: ^18.2.0 => 18.2.0 react-accessible-dropdown-menu-hook: ^3.2.0 => 3.2.0 react-country-region-selector: ^3.6.1 => 3.6.1 react-dom: ^18.2.0 => 18.2.0 react-draft-wysiwyg: ^1.15.0 => 1.15.0 react-feather: ^2.0.10 => 2.0.10 react-feature-flags: ^1.0.0 => 1.0.0 react-file-drop: ^3.1.5 => 3.1.5 react-graceful-image: ^1.5.0 => 1.5.0 react-helmet-async: ^1.3.0 => 1.3.0 react-hot-toast: ^2.3.0 => 2.3.0 react-html-parser: ^2.0.2 => 2.0.2 react-html-parser-demo: 0.0.0 react-loader-spinner: ^5.1.7-beta.1 => 5.1.7-beta.1 react-markdown: ^8.0.3 => 8.0.3 react-redux: ^8.0.2 => 8.0.2 react-router-dom: ^6.3.0 => 6.3.0 react-scripts: ^5.0.1 => 5.0.1 react-select: ^5.4.0 => 5.4.0 redux-persist: ^6.0.0 => 6.0.0 redux-persist/integration/react: undefined () sendbird-uikit: ^2.7.2 => 2.7.2 styled-components: ^5.3.5 => 5.3.5 styled-components/macro: undefined () styled-components/native: undefined () styled-components/primitives: undefined () styled-system: ^5.1.5 => 5.1.5 ts-morph: ^15.1.0 => 15.1.0 typescript: ^4.7.4 => 4.7.4 wait-on: ^6.0.1 => 6.0.1 web-vitals: ^2.1.4 => 2.1.4 webpack: 5 => 5.74.0 (4.46.0) npmGlobalPackages: @aws-amplify/cli: 9.2.0 @svgr/plugin-prettier: 6.3.1 commitizen: 4.2.5 corepack: 0.10.0 npm: 8.15.1 yarn: 1.22.19 ```

Describe the bug

I have a service that on signIn, it logs the user and returns allowing some animations to happen while we wait for the DataStore 'ready' event to take place, for that I use the Hub.listen('datastore', () => ...) sadly, in the process I let the issue for later and now it is much more harder to understand when/which change make this stop working.

signIn: async (email: Email, password: string, callback?: () => void) => {
    const cognitoUser: CognitoUserExt = await Auth.signIn(email, password)
    const profileService = new ProfileService({ userId: cognitoUser.attributes.sub })
    let profileExt: ProfileExt
    let professions: Profession[] = []
    let companies: Company[] = []

    console.log('Waiting for profile to be synced') // ---> gets called ✅

    // Keep the user in sync with the relevant data from Profile
    Hub.listen('datastore', async (hubData) => {
      console.log('@authentication loading Datastore', hubData.payload.event) // ---> Never gets called ❌
      const { event, data } = hubData.payload
     ...

      if (event === 'syncQueriesPartialSyncError') {
        notify.error('Error syncing data, please contact support at help@sol.earth')
      }

      // https://docs.amplify.aws/lib/datastore/datastore-events/q/platform/js/#ready
      if (event === 'ready') {
        console.log('@authentication DataStore is ready')
        // profileExt might not have the companies/professions at the moment the `Profile` model is synced
        dispatch(syncUser({ ...profileExt, professions, companies }))
        callback?.()
      }
    })

    // Allow to activate all subscriptions to sync cloud data
    await DataStore.start()
    analytics.identify(email)
    analytics.trackEvent('LOGIN')

    return true
  },

Expected behavior

I should receive events when listen for "datastore" scope under the Hub

Reproduction steps

I would imagine that by only trying to listen to Hub.listen('datastore') you might reproduce the issue but there is also https://github.com/aws-amplify/amplify-flutter/issues/1936#issuecomment-1252175413 that seems to claim on flutter they have a issue with models suffixed with Connection which I have.

Code Snippet

signIn: async (email: Email, password: string, callback?: () => void) => {
    const cognitoUser: CognitoUserExt = await Auth.signIn(email, password)
    const profileService = new ProfileService({ userId: cognitoUser.attributes.sub })
    let profileExt: ProfileExt
    let professions: Profession[] = []
    let companies: Company[] = []

    console.log('Waiting for profile to be synced') // ---> gets called ✅

    // Keep the user in sync with the relevant data from Profile
    Hub.listen('datastore', async (hubData) => {
      console.log('@authentication loading Datastore', hubData.payload.event) // ---> Never gets called ❌
      const { event, data } = hubData.payload
     ...

      if (event === 'syncQueriesPartialSyncError') {
        notify.error('Error syncing data, please contact support at help@sol.earth')
      }

      // https://docs.amplify.aws/lib/datastore/datastore-events/q/platform/js/#ready
      if (event === 'ready') {
        console.log('@authentication DataStore is ready')
        // profileExt might not have the companies/professions at the moment the `Profile` model is synced
        dispatch(syncUser({ ...profileExt, professions, companies }))
        callback?.()
      }
    })

    // Allow to activate all subscriptions to sync cloud data
    await DataStore.start()
    analytics.identify(email)
    analytics.trackEvent('LOGIN')

    return true
  },

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

I am using amplify 10.0.0

chrisbonifacio commented 1 year ago

Hi @duranmla 👋 thanks for raising this issue. Can you share your schema, or at least the models you believe might be part of the issue? I see you mentioned the flutter issue where they tested models with names such as Model{...}Connection but you referenced a model called SkillParticipantConnection which does not fit that format.

I noticed that you are establishing a listener in your signIn call, but are you also cleaning it up somewhere else? Perhaps on signOut? You may be experiencing this behavior because of duplicate/redundant listeners.

chrisbonifacio commented 1 year ago

I have not been able to reproduce the issue with the steps given. I tried creating two models with a Connection and Model{...}Connection naming format, but neither resulted in the issue described in the Flutter repo.

schema

type Todo @model {
  id: ID!
  name: String!
  description: String
}

type TodoConnection @model {
  id: ID!
  connected: Boolean
}

type ModelTodoConnection @model {
  id: ID!
  connected: Boolean
}

events

Screenshot 2022-09-21 at 4 11 48 PM

However, it's worth noting that the "ready" event was never emitted.

Upon checking the Amplify debugger logs, I can see that there are issues syncing the model named ModelTodoConnection

Screenshot 2022-09-21 at 4 17 37 PM

When inspecting the build schema, there is a manually created ModelTodoConnection type as well as a CLI generated ModelModelTodoConnectionConnection, which seems to be confusing the server and it expects to find the items, nextToken, and startedAt fields on the manually created model rather than the CLI generated model.

Screenshot 2022-09-21 at 4 32 21 PM

Considering this, I'm going to label this a bug for the Data team to investigate further. Until we come back with feedback, I would assume that the Model{...}Connection is reserved for the sync process and I would recommend avoiding it.

duranmla commented 1 year ago

Hi @duranmla 👋 thanks for raising this issue. Can you share your schema, or at least the models you believe might be part of the issue? I see you mentioned the flutter issue where they tested models with names such as Model{...}Connection but you referenced a model called SkillParticipantConnection which does not fit that format.

I noticed that you are establishing a listener in your signIn call, but are you also cleaning it up somewhere else? Perhaps on signOut? You may be experiencing this behavior because of duplicate/redundant listeners.

Hey, thanks for the reply, perhaps it is a problem with the duplicate/redundant listeners? I will take a look further once I finish some of the things I am doing, besides, I see also that you suggests that SkillParticipantConnection does not match the format Model{...}Connection, I though that when they said "Model" is any model name.

Either way, I have this issue on my app and another one with deployment that I need to come back later, I am just trying to move a little bit as much as I can while I got ideas from you folks.

Thanks a lot for the quick reply @chrisbonifacio

duranmla commented 1 year ago

One thing to mention with the comment of "redundant/duplicated" listeners is that I have a signOut method on my service like so:

signOut: async (options?: Object) => {
    await Auth.signOut(options)
    await DataStore.clear()
    dispatch(logOutUser())
    analytics.trackEvent('LOGOUT')
    analytics.reset()
  },

But how that can affect if it is not being trigger until the user click logout?

chrisbonifacio commented 1 year ago

I don't see that you are cleaning up the hub listener in either the sign in or sign out but you wouldn't accidentally set up more than one without signing in and out.

Can you share your schema? Or at least the models that seem to have this issue? You can also enable logging from Amplify by adding this line to your app:

Amplify.Logger.LOG_LEVEL = "DEBUG";

This should provide more insight into what's going on behind the scenes with DataStore, hopefully exposing some internal or network operations that are failing during the initial sync process.

duranmla commented 1 year ago

Sure, the schema I currently have is this one. https://gist.github.com/duranmla/5318da2f405d5b9d6baf0dfe36e170bd

duranmla commented 1 year ago

Also, I do have that logger level setup. Thanks for that anyway. I show the code at another issue I have at https://github.com/aws-amplify/amplify-js/issues/10382

UPDATE: I actually have INFO, I will change to DEBUG to check this out. Thanks for that.

🤔 I should maybe post that issue here tho.

chrisbonifacio commented 1 year ago

Hey @duranmla are you still experiencing this issue or have you been able to resolve it as well?

If the issue persists, please share the CLI version and schema so we can reproduce this more accurately?

chrisbonifacio commented 1 year ago

Transferring this issue over to the category-api repo because the return type for the sync query is not being computed correctly depending on the naming convention of a model that results in a naming collision during the sync

duranmla commented 1 year ago

Hey @chrisbonifacio not at all, the problem I had was seems to be caused by the fact I was running the DataStore.config twice https://github.com/aws-amplify/amplify-js/issues/10382#issuecomment-1265872505 thanks to your help I was able to get rid of this one.

Sorry for the delay to answer. It seems I have missed some Github notifications.

chrisbonifacio commented 1 year ago

@duranmla ah, no worries! Thanks for the update! I'll close this issue then and open a new one regarding the sync query error that results from naming models the same as a codegen type. I think we can improve the dev experience there.