-
Notifications
You must be signed in to change notification settings - Fork 49
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
Prohibiting SSE via POST breaks graphql-sse
usage (since 1.11.0)
#224
Comments
As mentioned on discord we never intended graphql-helix to be compatible with the protocol described via 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
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-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
@n1ru4l I think that would be best for now instead of closing the door completely. |
@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 |
Fixed in |
Since
v1.11.0
text/event-stream
/ SSE requests are forbidden viaPOST
:In
helix-flare
tests we are usinggraphql-sse
to create a subscription connection. It seems, thatgraphql-sse
usesPOST
andbody
to instantiate a connection which is now refused bygraphql-helix
.Tests are running fine for
helix-flare
with[email protected]
but are failing for1.11.0
.Am I wrong here by assuming that this should actually work fine with
POST
requests, or should I choose a different approach?The text was updated successfully, but these errors were encountered: