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

Getting "transport not supported" errors after updating to v0.17.50 #3275

Closed
mattjohnsonpint opened this issue Sep 14, 2024 · 3 comments · Fixed by #3276
Closed

Getting "transport not supported" errors after updating to v0.17.50 #3275

mattjohnsonpint opened this issue Sep 14, 2024 · 3 comments · Fixed by #3276

Comments

@mattjohnsonpint
Copy link

mattjohnsonpint commented Sep 14, 2024

Hi. We recently upgraded from v0.17.49 to v0.17.50, and all requests (queries/mutations) started failing with an error "transport not supported".

After some digging, I discovered the cause. #3153 added the following to the HTTP POST transport:

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

This is a problem because we just use handler.NewDefaultServer, which does not set up the SSE transport by default. Even if I add it with srv.AddTransport(transport.SSE{}), it still doesn't solve the problem because that would only be for subscriptions, not regular queries and mutations.

I was also surprised as for why the Accept header had text/event-stream for a non-subscription request in the first place. Turns out it was coming from our use of the popular urql client library, which does support GraphQL-SSE. And also, it 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',
  };

That might be in error - I'm not sure (I reported here: urql-graphql/urql#3673). But nonetheless, urql has > 1M downloads weekly - so this is bound to affect a lot of people.

From a general HTTP point of view though, the Accept header is there to communicate which content types that the client is able to understand, and indeed urql does understand SSE. IMHO, the presence of text/event-stream in the accept header shouldn't mean that the request can't be served by HTTP POST.

As a suggested fix, the SSE transport, if registered, would be used for subscription operations. Otherwise the normal list of transports should work as they would otherwise. The HTTP POST transport shouldn't have to remove itself from SSE requests.

@mattjohnsonpint
Copy link
Author

Per the response from urql maintainer, their behavior is by design and needed to support incremental delivery.

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Sep 15, 2024

Hey, I really appreciate you reporting this, especially with such a clear description and your detailed investigation. I reverted that change, and cut a new release.

@mattjohnsonpint
Copy link
Author

Thanks!

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