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

Be explicit how to handle unknown request information #1151

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Mar 27, 2025

As we are discussing adding a new request parameter for disabling error propagation, we'll need this. This should probably have been added 5 years ago but now is better than later. I think this is what most implementations are doing anyway.

Copy link

netlify bot commented Mar 27, 2025

Deploy Preview for graphql-spec-draft ready!

Name Link
🔨 Latest commit 64b5af3
🔍 Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/67ef065e87273a00083e4459
😎 Deploy Preview https://deploy-preview-1151--graphql-spec-draft.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@benjie benjie added the ✏️ Editorial PR is non-normative or does not influence implementation label Apr 3, 2025
@leebyron
Copy link
Collaborator

leebyron commented Apr 3, 2025

looks right - I'll tweak wording and then merge

@@ -20,6 +20,10 @@ Given this information, the result of {ExecuteRequest(schema, document,
operationName, variableValues, initialValue)} produces the response, to be
formatted according to the Response section below.

A GraphQL service must ignore unrecognized information in a request. This allows
Copy link
Member

Choose a reason for hiding this comment

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

This seems a little broad; arguably something inside the document (an unknown directive, a new syntactic symbol, fragment arguments) is in the request and could be unrecognized, and those should result in validation/parse errors rather than being ignored. Maybe we can use “additional” instead, such as “must ignore any additional unrecognised information” - that implies it’s outside of what’s already specified here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense to me 👍 . I know @leebyron wanted to make some wording changes, I'll defer to his decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ Editorial PR is non-normative or does not influence implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants