Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloudinary field doesn't work on Node 13.3.2 #2101

Closed
wesbos opened this issue Dec 13, 2019 · 16 comments · Fixed by #4588
Closed

Cloudinary field doesn't work on Node 13.3.2 #2101

wesbos opened this issue Dec 13, 2019 · 16 comments · Fixed by #4588
Assignees

Comments

@wesbos
Copy link
Contributor

wesbos commented Dec 13, 2019

Bug report

Not sure if this is upstream or not..

Trying to use the cloudinary field on Node 13.3.2. It gives the error (node:50514) [DEP0135] DeprecationWarning: ReadStream.prototype.open() is deprecated in the console and a 'nested error' in the Admin UI.

Reverting back to node 12.13 fixes it for now

System information

  • OS: Mac OS Catalina
@Vultraz
Copy link
Contributor

Vultraz commented Dec 14, 2019

What version of @keystonejs/file-adapters are you using?

@MadeByMike MadeByMike self-assigned this Dec 14, 2019
@wesbos
Copy link
Contributor Author

wesbos commented Dec 16, 2019

"@keystonejs/file-adapters": "^5.1.0",

@Vultraz
Copy link
Contributor

Vultraz commented Dec 17, 2019

Hm... I fixed the same warning in #1817 and I can't find where this instance may be coming from...

@wesbos
Copy link
Contributor Author

wesbos commented Jan 15, 2020

@Vultraz still broken on node 13.3.2. I think this is related to this:

jaydenseric/graphql-upload#170

@molomby
Copy link
Member

molomby commented Feb 3, 2020

Ah, pretty sure we hit this a few weeks ago too, right @MadeByMike? If my notes are to be believed it was a the combination of an outdated fs-capacitor package running on Node 13.

It manifested as a throw/catch loop, leading to a stack overflow. The loop traversed the following calls:

at ReadStream.<anonymous> (internal/fs/streams.js:147:3)
at ReadStream.deprecated [as open] (internal/util.js:89:15)
at ReadStream.open (/app/node_modules/fs-capacitor/lib/index.js:90:11)
at _openReadFs (internal/fs/streams.js:154:12)

fs-capacitor is a dependance of graphl-upload and apollo-upload-server. It's..

... a filesystem buffer for finite node streams.

apollo-server issue #3508 calls out the same problem. Looks like others have seen this in Keystone too, eg. #2254.

At the time, we fixed our deploy by switching to Node v12.4.1. I can't remember if there was a more general fix and my notes don't speak to it. @MadeByMike, do you recall what the outcome was? We probably bumped some packages or something..

@MadeByMike
Copy link
Contributor

Yes! This is totally that issue. It's an upstream problem and we changed our node version to fix it. Is there something more we can do to help users who might encounter this?

@JedWatson
Copy link
Member

@molomby @MadeByMike I believe that updated versions of the affected dependencies have all been released so we're not blocked, and now either:

a) we need to ensure those (and related) deps are updated and we release a new version of the affected packages; or
b) we've already done a) and this can be closed

Any chance someone can pick this up and confirm we've updated what we needed to, things have been released, and we can close this out?

@wesbos
Copy link
Contributor Author

wesbos commented Feb 25, 2020

Still an issue on node v13.9.0 with these deps:

    "@keystonejs/adapter-mongoose": "^5.2.0",
    "@keystonejs/app-admin-ui": "^5.8.0",
    "@keystonejs/app-graphql": "^5.1.0",
    "@keystonejs/app-next": "^5.1.0",
    "@keystonejs/auth-password": "^5.1.0",
    "@keystonejs/build-field-types": "^5.2.0",
    "@keystonejs/fields": "^6.3.0",
    "@keystonejs/fields-datetime-utc": "^5.1.0",
    "@keystonejs/fields-wysiwyg-tinymce": "^5.1.0",
    "@keystonejs/file-adapters": "^5.5.0",
    "@keystonejs/keystone": "^5.5.0",
    "@keystonejs/list-plugins": "^5.1.0",

Vultraz added a commit to Vultraz/keystone-5 that referenced this issue Mar 29, 2020
Fixes keystonejs#2101.

Currently, apollo-server-express depends on apollo-server-core which depends on graphql-upload ^8.0.2.
graphql-upload 9.0.0 updated its own fs-capacitor dependency to a newer version that supports Node 13,
but apollo-server-core hasn't updated its own graphql-upload dependency yet. Solve this by just forcing
a compatible version of graphql-upload (currently the latest 10.0.0) to be used.
@timleslie
Copy link
Contributor

Can confirm this is still an issue:

{
  errors: [
    {
      message: 'Nested errors occurred',
      name: 'NestedError',
      time_thrown: '2020-04-09T04:01:45.532Z',
      data: {
        errors: [
          {
            name: 'RangeError',
            message: 'Maximum call stack size exceeded',
            stack:
              'ReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:1:1)\nReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:94:11)\nReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:94:11)\nReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:94:11)\nReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:94:11)\nReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:94:11)\nReadStream.open (/Users/timleslie/src/keystone-5/node_modules/fs-capacitor/lib/index.js:94:11)\n',
          },
        ],
      },
      uid: 'ck8s8ir7d000badmu2v3ueivc',
    },
  ],
  data: { updateUser: null },
  extensions: {
    tracing: {
      version: 1,
      startTime: '2020-04-09T04:01:45.520Z',
      endTime: '2020-04-09T04:01:45.529Z',
      duration: 8665000,
      execution: {
        resolvers: [
          {
            path: ['updateUser'],
            parentType: 'Mutation',
            fieldName: 'updateUser',
            returnType: 'User',
            startOffset: 182591,
            duration: 8153392,
          },
        ],
      },
    },
  },
};

Node:

$ node --version
v13.12.0

package.json

  "dependencies": {
    "@apollo/react-hooks": "^3.1.3",
    "@apollo/react-ssr": "^3.1.3",
    "@emotion/core": "^10.0.27",
    "@keystonejs/adapter-mongoose": "^8.0.0",
    "@keystonejs/app-admin-ui": "^5.9.5",
    "@keystonejs/app-graphql": "^5.1.5",
    "@keystonejs/app-next": "^5.1.2",
    "@keystonejs/auth-password": "^5.1.5",
    "@keystonejs/email": "^5.1.2",
    "@keystonejs/fields": "^9.0.0",
    "@keystonejs/fields-wysiwyg-tinymce": "^5.2.3",
    "@keystonejs/file-adapters": "^6.0.1",
    "@keystonejs/keystone": "^8.0.0",
    "@keystonejs/session": "^6.0.1",
    "apollo-cache-inmemory": "^1.6.5",
    "apollo-client": "^2.6.8",
    "apollo-upload-client": "^12.1.0",
    "body-parser": "^1.18.2",
    "cross-env": "^7.0.0",
    "date-fns": "^1.30.1",
    "dotenv": "^8.2.0",
    "eslint-plugin-emotion": "^10.0.27",
    "express": "^4.17.1",
    "facepaint": "^1.2.1",
    "get-contrast": "^2.0.0",
    "graphql-tag": "^2.10.1",
    "isomorphic-unfetch": "^3.0.0",
    "lodash.uniqby": "^4.7.0",
    "next": "^9.3.2",
    "prop-types": "^15.7.2",
    "react": "^16.13.0",
    "react-dom": "^16.13.0",
    "react-toast-notifications": "^2.3.0",
    "react-use-form-state": "^0.12.0",
    "uuid": "^3.3.2"
  },

@MadeByMike
Copy link
Contributor

Reading more about this issue upstream I noticed this is working for some people: jaydenseric/graphql-upload#170 (comment)

@ralexrdz
Copy link
Contributor

going back to node v12.6 fixed it for me

@Bravo555
Copy link
Contributor

Bravo555 commented Aug 4, 2020

Here is a comment of apollo-server dev:
apollographql/apollo-server#3508 (comment)

Apollo will not be bumping graphql-upload, as they will not support it as of Apollo Server 3.0. Looks like the proper fix would be to disable the upload support bundled in ApolloServer, add graphql-upload as a dependency directly in packages that use it, and properly insert it as a middleware as described in the comment.

As graphql-upload is being removed in Apollo Server 3.0, this work will likely have to be done anyway.

Now, I tried looking in my project using Keystone where the broken fs-capacitor module originates from by npm ls fs-capacitor:

[email protected] /home/marcel/Documents/dev/projects/apartments-website
└─┬ @keystonejs/[email protected]
  └─┬ [email protected]
    └─┬ [email protected]
      └─┬ [email protected] invalid
        └── [email protected]  extraneous

(this is after applying a resolution as described above. without it versions of graphql-upload and fs-capacitor were respectively 8.1.0 and 2.0.4)

So graphql-upload is dependency of @keystonejs/keystone but from my limited understanding it doesnt use it directly. The fix is not trivial because I reckon @keystonejs/keystone shares the ApolloServer instance with @keystonejs/app-graphql and that's where this middleware business should be handled.

I'll try to look into it into more detail, in the meantime I'd be glad to have someone from the dev team review this and verify if this would be a viable solution.

@wesbos
Copy link
Contributor Author

wesbos commented Sep 30, 2020

This is still an issue for me. Not sure what happened but it was working on node 14.4, but then I upgraded to node v14.13.0, and it broke again.

Reverting back to v14.4 didn't fix it. Reverting to v12.4 did.

I also tried this in my package.json, but that doesn't seem to have done anything:

  "resolutions": {
    "graphql-upload": "^10.0.0"
  },
``

@wesbos
Copy link
Contributor Author

wesbos commented Sep 30, 2020

Fixed it! I was using npm, not yarn, which doesn't have resolutions.

So the fix is to use the above resolution with your package.json, but add this script:

"preinstall": "npx npm-force-resolutions"

@wesbos
Copy link
Contributor Author

wesbos commented Dec 4, 2020

This is kind of a pain because if someone deletes their package-lock.json file the preinstall doesn't work. They need to comment out the preinstall, run npm install, then comment it back in, then run npm install.

I know this is way upstream, but is there anything we can do to fix it in keystone?

@timleslie
Copy link
Contributor

Fixed in @keystonejs/[email protected].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants