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

Support sending operationName #86

Open
onionhammer opened this issue Oct 25, 2023 · 8 comments · May be fixed by #87
Open

Support sending operationName #86

onionhammer opened this issue Oct 25, 2023 · 8 comments · May be fixed by #87
Assignees

Comments

@onionhammer
Copy link

onionhammer commented Oct 25, 2023

OperationName is useful for logging purposes, frameworks like HotChocolate have a built-in field to handle it

image

Describe the solution you'd like
Pass operationName in the request payload

Describe alternatives you've considered
using a customFetch operation and extracting the operationName from the queries sent in

Additional context
Add any other context or screenshots about the feature request here.

@onionhammer onionhammer linked a pull request Oct 25, 2023 that will close this issue
@onionhammer
Copy link
Author

Hi @lynxtaa - is it possible to get this merged in the next couple months? If not I will have to figure out an alternative library/maintain a fork for now before I roll usage of this library into production - we currently rely on being able to see the operation name for monitoring performance of our queries

@lynxtaa
Copy link
Owner

lynxtaa commented Oct 31, 2023

Hi @onionhammer ! Thanks for the MR, I'll review it next week :)

@lynxtaa
Copy link
Owner

lynxtaa commented Oct 31, 2023

I have some troubles understanding why this feature is needed. The client doesn't need to send an operationName, the name of the operation would still be accessible on server after parsing the query. Right now I'm using Apollo Server and operationName is easily accessible for logging and tracing. Could you please provide more info?

@onionhammer
Copy link
Author

I have some troubles understanding why this feature is needed. The client doesn't need to send an operationName, the name of the operation would still be accessible on server after parsing the query. Right now I'm using Apollo Server and operationName is easily accessible for logging and tracing. Could you please provide more info?

Interesting, HotChocate server does not automatically extract that from the body of the query, I will do a little more research and get back to you tomorrow.

@onionhammer
Copy link
Author

Per the GraphQL Spec: https://spec.graphql.org/October2021/#GetOperation()

GetOperation(document, operationName)
If operationName is null:
If document contains exactly one operation.
Return the Operation contained in the document.
Otherwise raise a request error requiring operationName.
Otherwise:
Let operation be the Operation named operationName in document.
If operation was not found, raise a request error.
Return operation.

So it looks like as long as you send the operationName when multiple operations are being sent, this client should be good, otherwise it wont follow the spec

@lynxtaa
Copy link
Owner

lynxtaa commented Nov 10, 2023

Sorry for the delay @onionhammer . I see that we have two cases:

1. Single operation in a query

In that case the operationName field is not required and I don't see any valid reason to support it. I would suggest you getting it server-side from the query.

2. Multiple operations in a query

In that case the operationName field is required and the solution in #87 won't work. To support multiple queries we must somehow allow passing an additional operationName field when calling client.request(...) to allow the library user to choose the operation. At this moment I'm not sure how to implement it. As I see graphql-request have similar issue jasonkuhrt/graffle#64

@onionhammer
Copy link
Author

onionhammer commented Nov 10, 2023

The example I provided won't work 100% of the time, but the method in the pr would allow the user to put in the proper name of the operation

As far as my needs go, I have been able to extract the operation name from the server (for single queries)

@lynxtaa
Copy link
Owner

lynxtaa commented Nov 10, 2023

As far as my needs go, I have been able to extract the operation name from the server (for single queries)

That's great! Glad to hear it :)

Thanks for bringing this issue. When writing this lib I didn't know that the spec allows multiple operations in the same query. I'll keep this issue opened, maybe I'll manage to add the support for it later

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 a pull request may close this issue.

2 participants