wesbos commented 4 years ago

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 commented 4 years ago

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

wesbos commented 4 years ago

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

Vultraz commented 4 years ago

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

wesbos commented 4 years ago

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

https://github.com/jaydenseric/graphql-upload/issues/170

molomby commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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",

timleslie commented 4 years ago

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 commented 4 years ago

Reading more about this issue upstream I noticed this is working for some people: https://github.com/jaydenseric/graphql-upload/issues/170#issuecomment-641938198

ralexrdz commented 4 years ago

going back to node v12.6 fixed it for me

Bravo555 commented 4 years ago

Here is a comment of apollo-server dev: https://github.com/apollographql/apollo-server/issues/3508#issuecomment-662371289

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:

apartments-website@1.0.0 /home/marcel/Documents/dev/projects/apartments-website
└─┬ @keystonejs/keystone@13.0.0
  └─┬ apollo-server-express@2.16.1
    └─┬ apollo-server-core@2.16.1
      └─┬ graphql-upload@11.0.0 invalid
        └── fs-capacitor@6.2.0  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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

Fixed in @keystonejs/keystone@18.0.0.

keystonejs / keystone

Cloudinary field doesn't work on Node 13.3.2 #2101

Bug report

System information