awslabs / aws-mobile-appsync-sdk-js

JavaScript library files for Offline, Sync, Sigv4. includes support for React Native
Apache License 2.0
919 stars 266 forks source link

Cannot find module '@apollo/client' - introduced by v4.0.4 #647

Closed leonardomerlin closed 3 years ago

leonardomerlin commented 3 years ago

Note: If your issue/feature-request/question is regarding the AWS AppSync service, please log it in the official AWS AppSync forum

Do you want to request a feature or report a bug?

BUG

What is the current behavior?

Error: Cannot find module '@apollo/client'\nRequire stack:\n- /opt/nodejs/node_modules/aws-appsync-subscription-link/lib/subscription-handshake-link.js\n- /opt/nodejs/node_modules/aws-appsync-subscription-link/lib/index.js\n- /opt/nodejs/node_modules/aws-appsync/lib/deltaSync.js\n- /opt/nodejs/node_modules/aws-appsync/lib/cache/offline-cache.js\n- /opt/nodejs/node_modules/aws-appsync/lib/cache/index.js\n- /opt/nodejs/node_modules/aws-appsync/lib/client.js\n- /opt/nodejs/node_modules/aws-appsync/lib/index.js\n- /var/task/appsync/client.js\n- /var/task/middleware/appsync.js\n- /var/task/middleware/index.js\n- /var/task/handlers/appsyncExtractor.js\n- /var/task/handlers.js\n- /var/runtime/UserFunction.js\n- /var/runtime/index.js

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

Try to install the latest aws-appsync and run for production, it will break.

What is the expected behavior?

Do not break when using in production configuration

Which versions and which environment (browser, react-native, nodejs) / OS are affected by this issue? Did this work in previous versions?


I believe it was introduced in the recente MR: @apollo/client shouldn't be under devDependencies but under dependencies.

danrivett commented 3 years ago

I can confirm I have also run into this. My deployed lambdas were working fine on Wednesday, but not when I redeployed today.

Here is the stack trace I see in CloudWatch logs:

"stack": [
        "Runtime.ImportModuleError: Error: Cannot find module '@apollo/client'",
        "Require stack:",
        "- /var/task/node_modules/aws-appsync-subscription-link/lib/subscription-handshake-link.js",
        "- /var/task/node_modules/aws-appsync-subscription-link/lib/index.js",
        "- /var/task/node_modules/aws-appsync/lib/deltaSync.js",
        "- /var/task/node_modules/aws-appsync/lib/cache/offline-cache.js",
        "- /var/task/node_modules/aws-appsync/lib/cache/index.js",
        "- /var/task/node_modules/aws-appsync/lib/client.js",
        "- /var/task/node_modules/aws-appsync/lib/index.js",
        ...internal libary
]

After diffing the two lambda versions I deployed I also noticed the change from 4.0.3 to 4.0.4 appears to have caused this error.

One thing that really surprised me to see in a service release update of aws-appsync was seeing some of its dependencies be updated to a new major version as that seems risky for a service release?:

aws-appsync-auth-link: "^2.0.3" => "^3.0.4"
aws-appsync-subscription-link: "^2.2.1" => "^3.0.6"

I assume one or both of those changes is the cause? Which matches the finger being pointed to #646 above.

Although from the stack trace above the finger is pointed at aws-appsync-subscription-link, from the diff below of the two deployed lambdas, it appears both aws-appsync-auth-link and aws-appsync-subscription-link no longer includes apollo-link, which I can I assume causes this error?

lambda-diff

Copying @manueliglesias as the submitter of the suspect PR, and @elorzafe as the reviewer for both your analysis.

manueliglesias commented 3 years ago

Thank you for the detailed analysis, it helps a lot.

The introduction of @apollo/client as a devDependency was to get only the ApolloLink type (here), so the plan here will be to migrate to TypeScript 3.8 and make use of Type-Only imports so the import is removed from the transpiled code.

In the short term, you can do a yadd -D @apollo/client/npm i -D @apollo/client to install @apollo/client as a devDependency.

Regarding the dependencies updates:

aws-appsync-auth-link: "^2.0.3" => "^3.0.4"
aws-appsync-subscription-link: "^2.2.1" => "^3.0.6"

This was supposed to happen in a previous release but was missing.

danrivett commented 3 years ago

Thanks for the update @manueliglesias but I am unclear how adding a dev dependency on @apollo/client will help at runtime?

Maybe I don't understand this enough, is it perhaps because we're using webpack to build our lambdas and according to this webpack will still include the necessary dev dependency code?

If so, what about others who don't use webpack and just use tsc to build their lambdas?

Update: Thinking through this more, I'm not sure if webpack is even relevant here. We're using the Serverless framework to invoke webpack and build and deploy the lambda, so I assume it's under its remit to include or not @apollo/client and I would assume it doesn't include dev dependencies. But then again webpack will treeshake and avoid including unnecessary dependency code, so now I'm confused - an unexpected Friday night deployment problem has fried my brain after a long week, apologies.

Anyway, I'm currently adding @apollo/client as a regular dependency (since it's not really a dev dependency from what I can tell, given the stacktrace at runtime). I will see how that works. But any feedback or suggestions are most welcome as I'm following my nose on this error.

danrivett commented 3 years ago

My assumption is the upgrade to v 3.x of aws-appsync-subscription-link in particular has caused a dependency on apollo-link to no longer be present.

From looking at the dependencies of aws-appsync-subscription-link@2.2.1 and comparing with 3.0.6. I see that the dependency on apollo-link is gone, (which explains my diff above) but there is a new dev dependency on @apollo/client.

So I assume that v3 of these files require @apollo/client to be installed as a separate dependency. Which makes me wonder why it's not a peer dependency. Certainly I can see why those versions are a major version upgrade as they are breaking changes.

Which again makes me question why aws-appsync upgraded past a major version boundary in a service release if there are indeed breaking changes.

Again apologies if these are ignorant thoughts, but I'm struggling to find documentation to help me understand better.

danrivett commented 3 years ago

Anyway, I'm currently adding @apollo/client as a regular dependency ... I will see how that works.

Wow, well that just opened the can of worms wider - now I get the following runtime error - it wants React! I'm running in a Node 14 Lambda, there's definitely no React there.

[
        "Runtime.ImportModuleError: Error: Cannot find module 'react'",
        "Require stack:",
        "- /var/task/node_modules/@apollo/client/react/context/context.cjs.js",
        "- /var/task/node_modules/@apollo/client/react/react.cjs.js",
        "- /var/task/node_modules/@apollo/client/main.cjs.js",
        "- /var/task/node_modules/aws-appsync-subscription-link/lib/subscription-handshake-link.js",
        "- /var/task/node_modules/aws-appsync-subscription-link/lib/index.js",
        "- /var/task/node_modules/aws-appsync/lib/deltaSync.js",
        "- /var/task/node_modules/aws-appsync/lib/cache/offline-cache.js",
        "- /var/task/node_modules/aws-appsync/lib/cache/index.js",
        "- /var/task/node_modules/aws-appsync/lib/client.js",
        "- /var/task/node_modules/aws-appsync/lib/index.js",
        ...internal libary
]

This is getting concerning, and so I'll leave it to the experts to sort out. For now I'm going to pin our lambdas at aws-appsync@4.0.3 and avoid all this mess.

danrivett commented 3 years ago

Just confirming that pinning to 4.0.3 did indeed resolve the issue. Not a long term fix, but will tide me over until the long term fix is made known.

nabeelfarid commented 3 years ago

Just confirming that pinning to 4.0.3 did indeed resolve the issue. Not a long term fix, but will tide me over until the long term fix is made known.

Thanks for posting the issue and temp resolution. Spend hours yesterday trying to figure out what went wrong all of a sudden :)

tien commented 3 years ago

I was lucky enough to have 1 test case blow up from this instead of leaking it to production 🤦‍♂️

danrivett commented 3 years ago

@manueliglesias any update on this, as you can see from this ticket, there have been many affected by this bug, but there hasn't been any update from the AWS side acknowledging that this is something that needs to be, and will be, fixed on the AWS side, so I just wanted to check in.

We can pin to 4.0.2 in the short-medium term, but not the long term.

As mentioned by all of us, we are using aws-appsync in Lambda environments so that's our use case.

If there are better lighterweight alternatives to using that library, I would be open to that if it works with IAM permissions correctly and is straight forward or there are clear instructions.

Currently my setup below works very simply with the IAM permissions automatically set up for the Lambdas by the Serverless framework that deploy my lambda, which is why my following setup has worked really well and has been very simple to configure, up until 4.0.3:

import type { NormalizedCacheObject } from 'apollo-cache-inmemory';
import type { ApolloClientOptions } from 'apollo-client';
import AWSAppSyncClient, { AUTH_TYPE, AWSAppSyncClientOptions } from 'aws-appsync';
import AWS from 'aws-sdk/global';

const { APPSYNC_ENDPOINT, AWS_REGION } = process.env;

const { credentials } = AWS.config;

const appSyncConfig: AWSAppSyncClientOptions = {
  url: APPSYNC_ENDPOINT,
  region: AWS_REGION,
  auth: {
    type: AUTH_TYPE.AWS_IAM,
    credentials,
  },
  disableOffline: true,
};

const apolloClientOptions: Partial<ApolloClientOptions<NormalizedCacheObject>> = {
  defaultOptions: {
    query: {
      fetchPolicy: 'network-only', // important to make sure we get up-to-date results
      errorPolicy: 'all', // Helps log out errors returned from the AppSync GraphQL server
    },
  },
};

export const appSyncClient = new AWSAppSyncClient(appSyncConfig, apolloClientOptions);

The one downside is it does cause my Lambda to grow in size a lot, even when building with Webpack, and my lambda zip currently weighs in at around 18mb.

So if best practice is to avoid using aws-appsync in Lambdas we'd be open to that, but as I said it would need to work well, since the above has been very good for us until now and we haven't had to manually make low level HTTP requests or manually parse response bodies or anything.

manueliglesias commented 3 years ago

Hi,

First of all, I am sorry you had this ugly situation with the packages, after some deep investigation we have validated that things will work as expected.

We just published new versions of the packages, the packages were tested:

Some basic guidelines:


If there are better lighterweight alternatives to using that library, I would be open to that if it works with IAM permissions correctly and is straight forward or there are clear instructions.

@danrivett We provide instructions on how to call an AppSync GraphQL API from a Node.js app or a Lambda function, there you will also find information on how to sign the request using IAM credentials

Thank you for the patience