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

GraphQL Upload #30

Merged
merged 3 commits into from
Sep 13, 2021
Merged

GraphQL Upload #30

merged 3 commits into from
Sep 13, 2021

Conversation

alesso-x
Copy link
Contributor

@alesso-x alesso-x commented May 31, 2021

There are two GraphQLUpload scalars, the original from graphql-upload and the modified one from @graphql-tools/links

This PR reproduces an issue where the gateway schema modified scalar is not overriding the local schema version. See ardatan/graphql-tools#671 (comment). I copied the example from combining-local-and-remote-schemas and added graphql-upload.

When I use the original scalar in the local schema, I receive the following error:

Stacktrace

{
  "errors": [
    {
      "message": "Variable \"$file\" got invalid value {}; Upload value invalid.",
      "locations": [
        {
          "line": 5,
          "column": 15
        }
      ],
      "path": [
        "uploadFile"
      ],
      "extensions": {
        "code": "BAD_USER_INPUT",
        "exception": {
          "stacktrace": [
            "GraphQLError: Variable \"$file\" got invalid value {}; Upload value invalid.",
            "    at Object.relocatedError (/Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/utils/index.cjs.js:3960:12)",
            "    at mergeDataAndErrors (/Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/delegate/index.cjs.js:1400:34)",
            "    at checkResultAndHandleErrors (/Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/delegate/index.cjs.js:1389:38)",
            "    at CheckResultAndHandleErrors.transformResult (/Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/delegate/index.cjs.js:1385:16)",
            "    at /Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/delegate/index.cjs.js:1572:40",
            "    at Array.reduceRight (<anonymous>)",
            "    at Transformer.transformResult (/Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/delegate/index.cjs.js:1571:37)",
            "    at /Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/@graphql-tools/delegate/index.cjs.js:1657:48",
            "    at /Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/value-or-promise/build/main/ValueOrPromise.js:39:44",
            "    at new ValueOrPromise (/Users/alesso/workspace/schema-stitching-handbook/graphql-upload/node_modules/value-or-promise/build/main/ValueOrPromise.js:14:21)"
          ]
        }
      }
    }
  ],
  "data": null
}

To test, run the server yarn start and then run the following curl command.

curl localhost:4000/graphql \
  -F operations='{ "query": "mutation($file: Upload!) { uploadFile(input: $file) { filename mimetype content } }", "variables": { "file": null } }' \
  -F map='{ "0": ["variables.file"] }' \
  -F 0=@graphql-upload/file.txt

It will return the filename, mime type and content of the file. If you uncomment the original scalar in the file graphql-upload/services/local/schema.js, it will throw the error above.

Edit:

Added test, cd graphql-upload and run npx jest. Again it will pass with the modified scalar, if you change the local schema to use the original scalar, it will throw an error

Received: [[UserInputError: Variable "$input" got invalid value {}; Upload value invalid.]]

Comment on lines +3 to +7
// does not work
// const { GraphQLUpload } = require("graphql-upload");

// does work
const { GraphQLUpload } =require("@graphql-tools/links");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yaacovCR this is the issue I was having in this thread ardatan/graphql-tools#671 (comment). The original GraphQLUpload throws an error.

@gmac
Copy link
Owner

gmac commented Jun 16, 2021

Sorry I haven't gotten to this. I'll try to take a look sometime this week. Thanks for the contribution!

@gmac
Copy link
Owner

gmac commented Aug 29, 2021

So, to clarify, this is NOT a working example, it is an issue report opened in the form of a PR? This should be handled as an issue in GraphQL Tools linked to reproduction branches on your own fork. Please don’t PR code here until it is in good working order.

Closing this on the above assumption. Feel free to reopen when the issues are fixed, or if I’m misunderstanding the situation and the discussed issues have already been resolved.

@gmac gmac closed this Aug 29, 2021
@alesso-x
Copy link
Contributor Author

alesso-x commented Aug 29, 2021

So, to clarify, this is NOT a working example

The code does work, if you run the server and run the curl request, it will return the uploaded file.

This is probably the wrong place to add my issue but I just wanted to highlight that this version works but it might not be correct. The commented code is the correct version according to this comment but it's not working.

I think it's fine to close this PR since it's not correct. It would be nice to have an example of how to use graphql-upload with schema stitching.

@yaacovCR
Copy link
Collaborator

This is very helpful for the upstream issue. Not sure how urgent for you to debug as I see there is clear workaround, but the commented out code should be working as you mention.

@ghardin1314
Copy link

If anyone get stuck here in the future, be sure that you dont have batch: true in your SubschemaConfig. Hopefully I save someone some hours!

@gmac gmac reopened this Sep 5, 2021
@gmac gmac merged commit 60149ff into gmac:master Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants