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

Accept event-stream for POST for urql-graphql #3276

Merged
merged 1 commit into from
Sep 14, 2024

Conversation

StevenACoffman
Copy link
Collaborator

@StevenACoffman StevenACoffman commented Sep 14, 2024

Fix #3275

There was an attempt to support SSE, but it broke HTTP POST.

#3153 added the following to the HTTP POST transport:

if strings.Contains(r.Header.Get("Accept"), "text/event-stream") {
return false
}

From a general HTTP point of view though, the Accept header is there to communicate which content types the client can understand, and indeed clients like javascript's urql advertise that they can understand SSE, even when the server is only set up to support POST. The presence of text/event-stream in the client's accept header shouldn't mean that the request can't be served by HTTP POST.

For instance, the javascript client urql, has this header hardcoded for every request:

https://github.com/urql-graphql/urql/blob/b9f34fd19ee5f9022db4d2f9eb610943c787dac1/packages/core/src/internal/fetchOptions.ts#L149-L154

const headers: HeadersInit = {
    accept:
      operation.kind === 'subscription'
        ? 'text/event-stream, multipart/mixed'
        : 'application/graphql-response+json, application/graphql+json, application/json, text/event-stream, multipart/mixed',
  };

Signed-off-by: Steve Coffman steve@khanacademy.org

Signed-off-by: Steve Coffman <steve@khanacademy.org>
@coveralls
Copy link

Coverage Status

coverage: 56.727% (-0.01%) from 56.737%
when pulling 63a4e0c on accept_event_stream_POST
into 4157ef9 on master.

@StevenACoffman StevenACoffman merged commit ae51624 into master Sep 14, 2024
17 of 18 checks passed
@StevenACoffman StevenACoffman deleted the accept_event_stream_POST branch September 14, 2024 22:24
@StevenACoffman StevenACoffman added the SSE Server-Sent Events label Sep 15, 2024
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.

Getting "transport not supported" errors after updating to v0.17.50
2 participants