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

fix bug: POST transport Insufficient rigorous judgment leads to invalid SSE transport #3153

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

o8x
Copy link
Contributor

@o8x o8x commented Jun 22, 2024

fix bug: POST transfer judgment was not strict enough, resulting in SSE transport never will be invoked

For example, the following POST based SSE request will be automatically taken over by POST transport and will not enter the SSE process

curl -N --request POST --url http://localhost:8080/query \
--data '{"query":"subscription { currentTime { unixTime timeStamp } }"}' \
-H "accept: text/event-stream" -H 'content-type: application/json' \
--verbose

Easy bug fixes, without updating documentation

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@coveralls
Copy link

Coverage Status

coverage: 74.663% (+0.008%) from 74.655%
when pulling 448da58 on o8x:master
into 7c8bc50 on 99designs:master.

@StevenACoffman
Copy link
Collaborator

Hey, thank you very much! By the way, many users have asked me if there's a public repository that demonstrates using HTTP SSE with GraphQL. Are you able to make one public?

@StevenACoffman StevenACoffman merged commit feab5f5 into 99designs:master Jun 22, 2024
17 checks passed
@o8x
Copy link
Contributor Author

o8x commented Jun 23, 2024

Hey, thank you very much! By the way, many users have asked me if there's a public repository that demonstrates using HTTP SSE with GraphQL. Are you able to make one public?

@StevenACoffman
I would love to, but one issue prevented me from submitting the PR. I have submitted the details to the issue #3154

@StevenACoffman
Copy link
Collaborator

Thanks, finding this issue is also helpful! 😅

@StevenACoffman
Copy link
Collaborator

Hey, so it turns out this causes some problems with popular clients that advertise accepting text/event-stream, so I needed to revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SSE Server-Sent Events
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants