contra/graphql-helix

Prohibiting SSE via POST breaks `graphql-sse` usage (since 1.11.0)

dan-lee opened this issue ยท 6 comments

Since v1.11.0 text/event-stream / SSE requests are forbidden via POST:

POST requests that try to execute Subscription operations will now receive an error and 405 status code. This is not considered a breaking change as SSE is not doable over POST by the specification and was never officially supported.

In helix-flare tests we are using graphql-sse to create a subscription connection. It seems, that graphql-sse uses POST and body to instantiate a connection which is now refused by graphql-helix.

Tests are running fine for helix-flare with graphql-helix@1.10.3 but are failing for 1.11.0.

Am I wrong here by assuming that this should actually work fine with POST requests, or should I choose a different approach?

As mentioned on discord we never intended graphql-helix to be compatible with the protocol described via graphql-sse. The prior compatibility has been a pure coincidence.

The API for communicating with an SSE endpoint in browsers is EventSource. The EventSource ALWAYS sends a GET request to the SSE endpoint. There is no "official" SSE over HTTP spec yet. We made the assumption that SSE MUST be done over GET, however, this is assumption was wrong as the SSE specification does not really dictate the HTTP verb that should be used. It also seems like using GET for SSE is the common sense people agreed upon.

I don't really have an answer for this. I guess it might make sense to let people configure whether subscriptions are allowed via GET and/or POST?

This should probably also be a discussion on the GraphQL over HTTP repository

Same here, using graphql-sse on few projects and running into the same issue.
I'm a bit confused by such modification, it looks quite arbitrary to me to enforce Get over Post for SSE Subscription, and I thought the whole point of graphql-helix was to be

Framework and runtime agnostic. Use whatever HTTP library you want

Anyway, as a workaround I just did what I was already doing with apollo before, moving the subscription out of the graphql-helix endpoint, and use a dedicated subscription endpoint managed by graphql-sse directly

    // graphql-helix on `/graphql`
    this.app.use(`/graphql`, async (req, res) => {
      const request = { body: req.body, headers: req.headers, method: req.method, query: req.query }

     // ... graphql-helix code

      sendResult(result, res, formatResult)
    })

    // graphql-sse on `/stream`
    const handler = createHandler({ schema, context: (req: Request) => ({ db, pubsub, req }) })
    this.app.use(`/stream`, [...middlewares], handler)

and on client side, the only modification is to change the subscription url.

@kefniark My problem is that I am using Cloudflare Workers and the graphql-sse server handler relies on Node.js Request/Response APIs.

I don't really have an answer for this. I guess it might make sense to let people configure whether subscriptions are allowed via GET and/or POST?

@n1ru4l I think that would be best for now instead of closing the door completely.
I also think this needs a broader discussion to provide a more common standard.

@dan-lee @kefniark PR welcome for lifting the constraint ๐Ÿ˜‡ Sorry for causing the troubles, we discussed it internally ans we dont see why we should not allow it for now, as the official spec is still pretty vague (aka non-existing ๐Ÿ˜…)!

Slightly related: I would also appreciate any comments and thoughs on graphql/graphql-over-http#167

@n1ru4l I've opened PR #225 :)

Fixed in v1.12.0