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

v6: graphql() is not bound to client #12632

Closed johnf closed 4 months ago

johnf commented 10 months ago

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

GraphQL API

Amplify Categories

api

Environment information

``` System: OS: Linux 6.7 Ubuntu 23.10 23.10 (Mantic Minotaur) CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor Memory: 42.44 GB / 61.96 GB Container: Yes Shell: 5.2.15 - /bin/bash Binaries: Node: 18.18.2 - ~/.nvm/versions/node/v18.18.2/bin/node Yarn: 4.0.1 - ~/.nvm/versions/node/v18.18.2/bin/yarn npm: 9.8.1 - ~/.nvm/versions/node/v18.18.2/bin/npm Watchman: 4.9.0 - /usr/bin/watchman Browsers: Chrome: 119.0.6045.159 npmPackages: @amplitude/analytics-react-native: ^1.4.5 => 1.4.5 @aws-amplify/react-native: ^1.0.5 => 1.0.5 @aws-amplify/rtn-web-browser: ^1.0.5 => 1.0.5 @aws-sdk/client-cognito-identity: ^3.454.0 => 3.454.0 @aws-sdk/client-iam: ^3.454.0 => 3.454.0 @babel/cli: ^7.23.4 => 7.23.4 @babel/core: ^7.23.3 => 7.23.3 @babel/eslint-parser: ^7.23.3 => 7.23.3 @babel/preset-env: ^7.23.3 => 7.23.3 @babel/preset-typescript: ^7.23.3 => 7.23.3 @babel/runtime: ^7.23.4 => 7.23.4 @faker-js/faker: ^8.3.1 => 8.3.1 @googlemaps/polyline-codec: ^1.0.28 => 1.0.28 @gorhom/bottom-sheet: ^4.5.1 => 4.5.1 @hookform/resolvers: ^3.3.2 => 3.3.2 @hookform/resolvers/ajv: 1.0.0 @hookform/resolvers/arktype: 1.0.0 @hookform/resolvers/class-validator: 1.0.0 @hookform/resolvers/computed-types: 1.0.0 @hookform/resolvers/io-ts: 1.0.0 @hookform/resolvers/joi: 1.0.0 @hookform/resolvers/nope: 1.0.0 @hookform/resolvers/superstruct: 1.0.0 @hookform/resolvers/typanion: 1.0.0 @hookform/resolvers/typebox: 1.0.0 @hookform/resolvers/valibot: 1.0.0 @hookform/resolvers/vest: 1.0.0 @hookform/resolvers/yup: 1.0.0 @hookform/resolvers/zod: 1.0.0 @intercom/intercom-react-native: ^5.3.1 => 5.3.1 @notifee/react-native: ^7.8.0 => 7.8.0 @react-native-async-storage/async-storage: ^1.19.8 => 1.19.8 @react-native-camera-roll/camera-roll: ^6.0.2 => 6.0.2 @react-native-clipboard/clipboard: ^1.12.1 => 1.12.1 @react-native-community/netinfo: ^11.1.0 => 11.1.0 @react-native-firebase/app: ^18.6.2 => 18.6.2 @react-native-firebase/messaging: ^18.6.2 => 18.6.2 @react-native/eslint-config: ^0.72.2 => 0.72.2 @react-native/metro-config: ^0.73.2 => 0.73.2 @react-navigation/drawer: ^6.6.6 => 6.6.6 @react-navigation/native: ^6.1.9 => 6.1.9 @react-navigation/stack: ^6.3.20 => 6.3.20 @reeq/react-native-passkit: ^0.1.2 => 0.1.2 @rnx-kit/align-deps: ^2.2.6 => 2.2.6 @rnx-kit/babel-preset-metro-react-native: ^1.1.6 => 1.1.6 @rnx-kit/cli: ^0.16.20 => 0.16.20 @rnx-kit/metro-config: ^1.3.14 => 1.3.14 @rnx-kit/metro-resolver-symlinks: ^0.1.34 => 0.1.34 @sentry/cli: ^2.22.2 => 2.22.2 (2.21.3) @sentry/react-native: 5.14.1 => 5.14.1 @storybook/addon-essentials: ^7.5.3 => 7.5.3 @storybook/addon-interactions: ^7.5.3 => 7.5.3 @storybook/addon-links: ^7.5.3 => 7.5.3 @storybook/addon-react-native-web: ^0.0.21 => 0.0.21 @storybook/blocks: ^7.5.3 => 7.5.3 @storybook/react: ^7.5.3 => 7.5.3 @storybook/react-webpack5: ^7.5.3 => 7.5.3 @storybook/testing-library: ^0.2.2 => 0.2.2 @stripe/stripe-react-native: 0.33.0 => 0.33.0 @survicate/react-native-survicate: https://github.com/johnf/react-native-survicate.git#add-events => 3.0.2 @tanstack/eslint-plugin-query: ^5.8.4 => 5.8.4 @tanstack/query-codemods: 4.24.3 @tanstack/react-query: ^5.8.7 => 5.8.7 @total-typescript/ts-reset: ^0.5.1 => 0.5.1 @tsconfig/node18-strictest-esm: ^1.0.1 => 1.0.1 @tsconfig/react-native: ^3.0.2 => 3.0.2 @types/google.maps: ^3.54.9 => 3.54.9 @types/metro-config: ^0.76.3 => 0.76.3 @types/qrcode: ^1.5.5 => 1.5.5 @types/react: ^18.2.38 => 18.2.38 @types/react-native-get-random-values: ^1.8.2 => 1.8.2 @types/react-native-vector-icons: ^6.4.18 => 6.4.18 @types/react-native-version-check: ^3.4.8 => 3.4.8 @types/react-test-renderer: ^18.0.7 => 18.0.7 @types/survicate__react-native-survicate: ^1.1.2 => 1.1.2 @types/uuid: ^9.0.7 => 9.0.7 @typescript-eslint/eslint-plugin: ^6.12.0 => 6.12.0 (5.62.0) @typescript-eslint/parser: ^6.12.0 => 6.12.0 (5.62.0) HelloWorld: 0.0.1 appcenter-cli: ^2.14.0 => 2.14.0 awesome-phonenumber: ^6.2.0 => 6.2.0 aws-amplify: ^6.0.5 => 6.0.5 aws-amplify/adapter-core: undefined () aws-amplify/analytics: undefined () aws-amplify/analytics/kinesis: undefined () aws-amplify/analytics/kinesis-firehose: undefined () aws-amplify/analytics/personalize: undefined () aws-amplify/analytics/pinpoint: undefined () aws-amplify/api: undefined () aws-amplify/api/server: undefined () aws-amplify/auth: undefined () aws-amplify/auth/cognito: undefined () aws-amplify/auth/cognito/server: undefined () aws-amplify/auth/server: undefined () aws-amplify/datastore: undefined () aws-amplify/in-app-messaging: undefined () aws-amplify/in-app-messaging/pinpoint: undefined () aws-amplify/push-notifications: undefined () aws-amplify/push-notifications/pinpoint: undefined () aws-amplify/storage: undefined () aws-amplify/storage/s3: undefined () aws-amplify/storage/s3/server: undefined () aws-amplify/storage/server: undefined () aws-amplify/utils: undefined () babel-jest: ^29.7.0 => 29.7.0 babel-loader: ^9.1.3 => 9.1.3 babel-plugin-inline-import: ^3.0.0 => 3.0.0 babel-plugin-module-resolver: ^5.0.0 => 5.0.0 babel-plugin-react-native-web: ^0.19.9 => 0.19.9 chromatic: ^9.1.0 => 9.1.0 currency-symbol-map: ^5.1.0 => 5.1.0 dayjs: ^1.11.10 => 1.11.10 eslint: ^8.54.0 => 8.54.0 eslint-config-airbnb: ^19.0.4 => 19.0.4 eslint-config-airbnb-typescript: ^17.1.0 => 17.1.0 eslint-import-resolver-alias: ^1.1.2 => 1.1.2 eslint-import-resolver-typescript: ^3.6.1 => 3.6.1 eslint-plugin-import: ^2.29.0 => 2.29.0 eslint-plugin-jsx-a11y: ^6.8.0 => 6.8.0 eslint-plugin-react: ^7.33.2 => 7.33.2 eslint-plugin-react-hooks: ^4.6.0 => 4.6.0 eslint-plugin-react-native: ^4.1.0 => 4.1.0 eslint-plugin-security-node: ^1.1.1 => 1.1.1 eslint-plugin-storybook: ^0.6.15 => 0.6.15 expect-telnet: ^1.0.0 => 1.0.0 getstream: ^8.4.1 => 8.4.1 glob: ^10.3.10 => 10.3.10 (7.2.3, 7.2.0, 8.1.0, 7.1.6) icon-set-creator: ^1.2.6 => 1.2.6 is-european: ^1.0.7 => 1.0.7 jest: ^29.7.0 => 29.7.0 jotai: ^2.6.0 => 2.6.0 metro-react-native-babel-preset: ^0.77.0 => 0.77.0 (0.76.8) minisearch: ^6.3.0 => 6.3.0 native-base: ^3.4.28 => 3.4.28 nativewind: 4.0.13 => 4.0.13 patch-package: ^8.0.0 => 8.0.0 postinstall-postinstall: ^2.1.0 => 2.1.0 prop-types: ^15.8.1 => 15.8.1 qrcode: ^1.5.3 => 1.5.3 query-string: ^8.1.0 => 8.1.0 (7.1.3) react: 18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 react-hook-form: ^7.48.2 => 7.48.2 react-native: 0.72.7 => 0.72.7 react-native-animated-pagination-dots: ^0.1.73 => 0.1.73 react-native-avoid-softinput: ^4.0.2 => 4.0.2 react-native-bootsplash: ^5.1.3 => 5.1.3 react-native-branch: ^6.0.0 => 6.0.0 react-native-calendars: ^1.1302.0 => 1.1302.0 react-native-clean-project: ^4.0.1 => 4.0.1 react-native-code-push: ^8.1.0 => 8.1.0 react-native-color-matrix-image-filters: ^6.0.3 => 6.0.3 react-native-confirmation-code-field: ^7.3.2 => 7.3.2 react-native-country-codes-picker: ^2.3.4 => 2.3.4 react-native-date-picker: ^4.3.3 => 4.3.3 react-native-device-info: ^10.11.0 => 10.11.0 react-native-dotenv: ^3.4.9 => 3.4.9 react-native-dropdown-picker: ^5.4.6 => 5.4.6 react-native-geolocation-service: ^5.3.1 => 5.3.1 react-native-gesture-handler: ^2.14.0 => 2.14.0 react-native-get-random-values: ^1.10.0 => 1.10.0 react-native-image-crop-picker: ^0.40.2 => 0.40.2 react-native-inappbrowser-reborn: ^3.7.0 => 3.7.0 react-native-linear-gradient: ^2.8.3 => 2.8.3 react-native-localize: ^3.0.4 => 3.0.4 react-native-logs: ^5.0.1 => 5.0.1 react-native-maps: 1.8.0 => 1.8.0 react-native-permissions: ^3.10.1 => 3.10.1 react-native-reanimated: ^3.5.4 => 3.5.4 react-native-reanimated-carousel: ^3.5.1 => 3.5.1 react-native-safe-area-context: ^4.7.4 => 4.7.4 react-native-screens: ^3.27.0 => 3.27.0 react-native-svg: ^13.14.0 => 13.14.0 react-native-url-polyfill: ^2.0.0 => 2.0.0 react-native-vector-icons: ^10.0.2 => 10.0.2 react-native-version-check: ^3.4.7 => 3.4.7 react-native-web: ^0.19.9 => 0.19.9 react-test-renderer: 18.2.0 => 18.2.0 storybook: ^7.5.3 => 7.5.3 tailwindcss: ^3.3.5 => 3.3.5 ts-node: ^11.0.0-beta.1 => 11.0.0-beta.1 typescript: ^5.3.2 => 5.3.2 uuid: ^9.0.1 => 9.0.1 (8.3.2, 7.0.3, 3.4.0) webpack: ^5.89.0 => 5.89.0 webpack-cli: ^5.1.4 => 5.1.4 (4.9.2) yup: ^1.3.2 => 1.3.2 yup-phone: ^1.3.2 => 1.3.2 npmGlobalPackages: @aws-amplify/cli: 12.8.2 aws-cdk: 2.110.1 corepack: 0.19.0 neovim: 4.10.1 npm: 9.8.1 ```

Describe the bug

I'm seeing the following error when trying to use the graphql API and API calls fail.

 ERROR  ERROR {"error": [TypeError: Cannot read property 'getConfig' of undefined]}

This occurs at https://github.com/aws-amplify/amplify-js/blob/main/packages/api-graphql/src/utils/resolveConfig.ts#L13 in resoveConfig

I've tracked the core issue down to https://github.com/aws-amplify/amplify-js/blob/main/packages/api-graphql/src/internals/v6.ts#L115-L121

It is passing the amplify object as this[___amplify] but the client hasn't been bound to the graphql function

A quick fix is to bind it at https://github.com/aws-amplify/amplify-js/blob/main/packages/api-graphql/src/internals/generateClient.ts#L23-L40

export function generateClient<T extends Record<any, any> = never>(
    params: ClientGenerationParams
): V6Client<T> {
    const client = {
        [__amplify]: params.amplify,
        [__authMode]: params.authMode,
        [__authToken]: params.authToken,
        [__headers]: params.headers,
                // JF CHANGE: Comment his out
                // graphql,
        cancel,
        isCancelError,
        models: {},
    } as any;

        // JF CHANGE: Bind the function
    client.graphql = graphql.bind(client);

    client.models = generateModelsProperty<T>(client, params);

    return client as V6Client<T>;

It's quite possible I'm doing something wrong as I'm not sure how this could work for anyone as is

Expected behavior

API calls succeed without any issues.

Reproduction steps

Set up a standard amplify app and use a graphql query

Code Snippet

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

johnf commented 10 months ago

This is the patch I'm using

diff --git a/node_modules/@aws-amplify/api-graphql/src/internals/generateClient.ts b/node_modules/@aws-amplify/api-graphql/src/internals/generateClient.ts
index 7bf63b9..b0dc4e3 100644
--- a/node_modules/@aws-amplify/api-graphql/src/internals/generateClient.ts
+++ b/node_modules/@aws-amplify/api-graphql/src/internals/generateClient.ts
@@ -28,12 +28,13 @@ export function generateClient<T extends Record<any, any> = never>(
        [__authMode]: params.authMode,
        [__authToken]: params.authToken,
        [__headers]: params.headers,
-       graphql,
        cancel,
        isCancelError,
        models: {},
    } as any;

+   client.graphql = graphql.bind(client);
+
    client.models = generateModelsProperty<T>(client, params);

    return client as V6Client<T>;
david-mcafee commented 10 months ago

@johnf - Could you share code snippets demonstrating where / how you are calling both Amplify.configure and generateClient? Thank you!

johnf commented 10 months ago

Sure.

NOTE: For completeness I've added my retry wrapper below, but I had the same issue without the wrapper


// App.tsx
import { Amplify } from 'aws-amplify'; 
import amplifyconfig from '../amplifyconfiguration.json';                                                                                                                                                      

// Fix the oauth redirects - DOn't think this is relevant but including it anyway
amplifyconfig.oauth.redirectSignIn = 'xxx';
amplifyconfig.oauth.redirectSignOut = 'xxx';
amplifyconfig.oauth.domain = 'xxx';

Amplify.configure(amplifyconfig);

// models/User.ts
import { graphql, GeneratedQuery } from '../core/graphql';

const getUser = /* GraphQL */ `
  query GetUser($id: ID!) {
    getUser(id: $id) {
      ${userObject}
  }
` as GeneratedQuery<GetUserQueryVariables, GetUserQuery>;

export const find = async (id: string) => {
  // console.debug(`User.find(${id})`);

  const response = await graphql({
    query: getUser,
    variables: { id },
  });

  const user = response.data?.getUser as User | undefined;
  if (!user) {
    return null;
  }

  return transformUser(user);
};

// grapql.ts - My abstraction
import { generateClient } from 'aws-amplify/api';

// NOTE: Create an issue upstream to export this type
export type GeneratedQuery<InputType, OutputType> = string & {
  __generatedQueryInput: InputType;
  __generatedQueryOutput: OutputType;
};

const client = generateClient();

// Define a generic type for a function
type Func<T extends any[], R> = (...args: T) => R; // eslint-disable-line @typescript-eslint/no-explicit-any

const wait = (ms: number) => new Promise((res) => { setTimeout(res, ms); });

const maxRetries = 3;

const retry = <T extends any[], R>(originalFunction: Func<T, R>): Func<T, Promise<R>> => async (...args: T) => { // eslint-disable-line @typescript-eslint/no-explicit-any
  let retries = 0;
  let error: Error;

  for (;;) {
    try {
      return originalFunction(...args);
    } catch (err) {
      console.debug(err);
      retries += 1;

      const errors = (err as { errors: Error[] }).errors || [];
      [error] = errors;

      if (error && error.message === 'Network Error') {
        if (retries > maxRetries) {
          Sentry.captureMessage('GraphQL Network Error: Too many retries', { extra: { json: JSON.stringify(err), retries } });

          throw err;
        }

        console.error('We got a network error, we should retry', retries);
        await wait(2 ** retries * 10); // eslint-disable-line no-await-in-loop
      } else {
        // If max retries reached, throw the last error
        console.error('GraphQL Error2', err);
        Sentry.captureMessage(`GraphQL Error ${args[0]}`, { extra: { errors: JSON.stringify(errors), retries, data: JSON.stringify(err, null, 2).slice(0, 200) } });

        throw err;
      }
    }
  }
};

export const graphql = retry(client.graphql);
chrisbonifacio commented 10 months ago

Hi @johnf, thank you for the code examples!

I was able to reproduce the issue locally and have created a sample app for the team to use to investigate the issue further.

https://github.com/chrisbonifacio/v6-repro-12632

cc @david-mcafee

chrisbonifacio commented 10 months ago

@johnf @david-mcafee

Want to note that I also happened to observe successful API calls if I moved the client generation to the App.tsx file, after the Amplify.configure call.

CleanShot 2023-11-28 at 22 01 52@2x

simplified App.tsx

/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 *
 * @format
 */

import React from 'react';
import {Button, SafeAreaView, StatusBar, useColorScheme} from 'react-native';

import {Colors} from 'react-native/Libraries/NewAppScreen';

import {Amplify} from 'aws-amplify';
import amplifyconfig from './src/amplifyconfiguration.json';
import {generateClient} from 'aws-amplify/api';
import {createUser} from './src/graphql/mutations';

Amplify.configure(amplifyconfig);

const client = generateClient();

function App(): JSX.Element {
  const isDarkMode = useColorScheme() === 'dark';

  const backgroundStyle = {
    backgroundColor: isDarkMode ? Colors.darker : Colors.lighter,
  };

  return (
    <SafeAreaView style={backgroundStyle}>
      <StatusBar
        barStyle={isDarkMode ? 'light-content' : 'dark-content'}
        backgroundColor={backgroundStyle.backgroundColor}
      />
      <Button
        title="Create User"
        onPress={async () => {
          const user = await client.graphql({
            query: createUser,
            variables: {
              input: {
                username: 'cbonif',
              },
            },
          });

          console.log({user});
        }}
      />
    </SafeAreaView>
  );
}

export default App;
stelmakhivan commented 10 months ago

@chrisbonifacio, when are you going to release the fix? Having the same issue, but want to avoid patch-package usage for now...

svidgen commented 10 months ago

Hey @johnf, it looks like you're creating a wrapper that ultimately invokes client.graphql() separately from the intended instance context. I.e., the this that the graphql() method sees in your call chain would be the global object. And, that wouldn't be expected to function correctly. This would explain why adding that .bind() solves the issue for you.

A future-proof solution I've seen a lot for this type of problem is to create a simple "intermediate" lambda whenever you're creating a "reference" to an instance method. That is, use () => a.b() whenever you're tempted to pass method a.b around. FWIW, in my own code, unless I know that an object is designed for me to refer directly to its instance methods to be able to call them "out of context", I will defensively create these wrappers just about every time.

In your code, I think you'd need to change this line:

export const graphql = retry(client.graphql);

To something like this:

export const graphql = retry(((...args) => client.graphql(...args)) as typeof client.graphql);

Can you try that and let us know if that solves the problem for you?

(Assuming I've diagnosed this correctly (pending your response), we'll need to chat internally about if and how we can broadly support the call pattern you're after here.)

johnf commented 8 months ago

@svidgen I can confirm that does fix it.

arantespp commented 8 months ago

I got this error in my monorepo with PNPM when some packages used Amplify v5 and others, v6. Once I removed the packages using Amplify v5, this error was gone.

chrisbonifacio commented 4 months ago

Closing this issue as the original author is unblocked and this does not seem to be a bug.

Others running into this issue or similar, please refer to https://github.com/aws-amplify/amplify-js/issues/12632#issuecomment-1856314608