Skip to content
This repository was archived by the owner on Dec 20, 2024. It is now read-only.

Commit f4399bb

Browse files
authored
fix: broken GraphiQL implementation + GraphiQL puppeteer tests (#83)
* add cypress tests * update url loader * add changeset * flip condition * fix assertions * add use legacy ws protcol option * replace cypress with puppeteer * remove cypress.json * remove dev server * remove ts-node-dev * remove unused file * fix graphiql and correctly terminate sse streams * ensure SSE connection is closed
1 parent 1f5eeda commit f4399bb

File tree

14 files changed

+9703
-10001
lines changed

14 files changed

+9703
-10001
lines changed

.changeset/poor-spiders-own.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"graphql-helix": patch
3+
---
4+
5+
terminate SSE HTTP connection after stream ended emitting values

.changeset/wet-rules-approve.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"graphql-helix": patch
3+
---
4+
5+
fix broken multi part response and SSE response fetching in GraphiQL

packages/core/lib/send-result/node-http.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ export async function sendPushResult(
7878
// @ts-expect-error - Different Signature between ServerResponse and Http2ServerResponse but still compatible.
7979
rawResponse.write(`data: ${JSON.stringify(transformResult(result))}\n\n`);
8080
});
81+
rawResponse.end();
8182
}
8283

8384
export async function sendResult(

packages/core/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
"jest": "27.2.1",
5656
"lint-staged": "11.1.2",
5757
"move-file-cli": "3.0.0",
58+
"puppeteer": "10.4.0",
5859
"replacestream": "4.0.3",
5960
"ts-jest": "27.0.5",
6061
"ts-node": "10.2.1",

packages/core/test/implementations/express.ts

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import express, { RequestHandler } from "express";
2-
import { getGraphQLParameters, processRequest, renderGraphiQL, shouldRenderGraphiQL } from "../../lib";
2+
import { getGraphQLParameters, processRequest, renderGraphiQL, sendResult, shouldRenderGraphiQL } from "../../lib";
33
import { schema } from "../schema";
44

55
const graphqlMiddleware: RequestHandler = async (req, res) => {
@@ -18,51 +18,7 @@ const graphqlMiddleware: RequestHandler = async (req, res) => {
1818
schema,
1919
});
2020

21-
if (result.type === "RESPONSE") {
22-
result.headers.forEach(({ name, value }) => res.setHeader(name, value));
23-
res.status(result.status);
24-
res.json(result.payload);
25-
} else if (result.type === "PUSH") {
26-
res.writeHead(200, {
27-
"Content-Type": "text/event-stream",
28-
Connection: "keep-alive",
29-
"Cache-Control": "no-cache",
30-
});
31-
32-
req.on("close", () => {
33-
result.unsubscribe();
34-
});
35-
36-
await result.subscribe((result) => {
37-
res.write(`data: ${JSON.stringify(result)}\n\n`);
38-
});
39-
} else {
40-
res.writeHead(200, {
41-
Connection: "keep-alive",
42-
"Content-Type": 'multipart/mixed; boundary="-"',
43-
"Transfer-Encoding": "chunked",
44-
});
45-
46-
req.on("close", () => {
47-
result.unsubscribe();
48-
});
49-
50-
res.write("---");
51-
52-
await result.subscribe((result) => {
53-
const chunk = Buffer.from(JSON.stringify(result), "utf8");
54-
const data = ["", "Content-Type: application/json; charset=utf-8", "Content-Length: " + String(chunk.length), "", chunk];
55-
56-
if (result.hasNext) {
57-
data.push("---");
58-
}
59-
60-
res.write(data.join("\r\n"));
61-
});
62-
63-
res.write("\r\n-----\r\n");
64-
res.end();
65-
}
21+
await sendResult(result, res);
6622
};
6723

6824
const graphiqlMiddleware: RequestHandler = async (_req, res) => {

packages/core/test/implementations/fastify.ts

Lines changed: 4 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
import fastify, { RouteHandlerMethod } from "fastify";
2-
import {
3-
getGraphQLParameters,
4-
processRequest,
5-
renderGraphiQL,
6-
shouldRenderGraphiQL,
7-
} from "../../lib";
2+
import { getGraphQLParameters, processRequest, renderGraphiQL, sendResult, shouldRenderGraphiQL } from "../../lib";
83
import { schema } from "../schema";
94

105
const graphqlHandler: RouteHandlerMethod = async (req, res) => {
@@ -23,57 +18,9 @@ const graphqlHandler: RouteHandlerMethod = async (req, res) => {
2318
schema,
2419
});
2520

26-
if (result.type === "RESPONSE") {
27-
result.headers.forEach(({ name, value }) => res.header(name, value));
28-
res.status(result.status);
29-
res.send(result.payload);
30-
} else if (result.type === "PUSH") {
31-
res.raw.writeHead(200, {
32-
"Content-Type": "text/event-stream",
33-
Connection: "keep-alive",
34-
"Cache-Control": "no-cache",
35-
});
36-
37-
req.raw.on("close", () => {
38-
result.unsubscribe();
39-
});
40-
41-
await result.subscribe((result) => {
42-
res.raw.write(`data: ${JSON.stringify(result)}\n\n`);
43-
});
44-
} else {
45-
res.raw.writeHead(200, {
46-
Connection: "keep-alive",
47-
"Content-Type": 'multipart/mixed; boundary="-"',
48-
"Transfer-Encoding": "chunked",
49-
});
50-
51-
req.raw.on("close", () => {
52-
result.unsubscribe();
53-
});
54-
55-
res.raw.write("---");
56-
57-
await result.subscribe((result) => {
58-
const chunk = Buffer.from(JSON.stringify(result), "utf8");
59-
const data = [
60-
"",
61-
"Content-Type: application/json; charset=utf-8",
62-
"Content-Length: " + String(chunk.length),
63-
"",
64-
chunk,
65-
];
66-
67-
if (result.hasNext) {
68-
data.push("---");
69-
}
70-
71-
res.raw.write(data.join("\r\n"));
72-
});
73-
74-
res.raw.write("\r\n-----\r\n");
75-
res.raw.end();
76-
}
21+
await sendResult(result, res.raw);
22+
// Tell fastify a response was sent
23+
res.sent = true;
7724
};
7825

7926
const graphiqlHandler: RouteHandlerMethod = async (_req, res) => {

packages/core/test/implementations/fastify_async.ts

Lines changed: 5 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
import fastify, { RouteHandlerMethod } from "fastify";
22
import { parse as graphqlParse } from "graphql";
3-
import {
4-
getGraphQLParameters,
5-
processRequest,
6-
renderGraphiQL,
7-
shouldRenderGraphiQL,
8-
} from "../../lib";
3+
import { getGraphQLParameters, processRequest, renderGraphiQL, sendResult, shouldRenderGraphiQL } from "../../lib";
94
import { schema } from "../schema";
105

11-
const sleep = (time: number) =>
12-
new Promise<void>((resolve) => setTimeout(resolve, time));
6+
const sleep = (time: number) => new Promise<void>((resolve) => setTimeout(resolve, time));
137

148
const graphqlHandler: RouteHandlerMethod = async (req, res) => {
159
const request = {
@@ -31,57 +25,9 @@ const graphqlHandler: RouteHandlerMethod = async (req, res) => {
3125
},
3226
});
3327

34-
if (result.type === "RESPONSE") {
35-
result.headers.forEach(({ name, value }) => res.header(name, value));
36-
res.status(result.status);
37-
res.send(result.payload);
38-
} else if (result.type === "PUSH") {
39-
res.raw.writeHead(200, {
40-
"Content-Type": "text/event-stream",
41-
Connection: "keep-alive",
42-
"Cache-Control": "no-cache",
43-
});
44-
45-
req.raw.on("close", () => {
46-
result.unsubscribe();
47-
});
48-
49-
await result.subscribe((result) => {
50-
res.raw.write(`data: ${JSON.stringify(result)}\n\n`);
51-
});
52-
} else {
53-
res.raw.writeHead(200, {
54-
Connection: "keep-alive",
55-
"Content-Type": 'multipart/mixed; boundary="-"',
56-
"Transfer-Encoding": "chunked",
57-
});
58-
59-
req.raw.on("close", () => {
60-
result.unsubscribe();
61-
});
62-
63-
res.raw.write("---");
64-
65-
await result.subscribe((result) => {
66-
const chunk = Buffer.from(JSON.stringify(result), "utf8");
67-
const data = [
68-
"",
69-
"Content-Type: application/json; charset=utf-8",
70-
"Content-Length: " + String(chunk.length),
71-
"",
72-
chunk,
73-
];
74-
75-
if (result.hasNext) {
76-
data.push("---");
77-
}
78-
79-
res.raw.write(data.join("\r\n"));
80-
});
81-
82-
res.raw.write("\r\n-----\r\n");
83-
res.raw.end();
84-
}
28+
await sendResult(result, res.raw);
29+
// Tell fastify a response was sent
30+
res.sent = true;
8531
};
8632

8733
const graphiqlHandler: RouteHandlerMethod = async (_req, res) => {

packages/core/test/implementations/koa.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
import Koa, { Context } from "koa";
22
import bodyParser from "koa-bodyparser";
33
import { PassThrough } from "stream";
4-
import {
5-
getGraphQLParameters,
6-
processRequest,
7-
renderGraphiQL,
8-
shouldRenderGraphiQL,
9-
} from "../../lib";
4+
import { getGraphQLParameters, processRequest, renderGraphiQL, shouldRenderGraphiQL } from "../../lib";
105
import { schema } from "../schema";
116

127
const graphqlHandler = async (ctx: Context) => {
@@ -50,9 +45,13 @@ const graphqlHandler = async (ctx: Context) => {
5045
ctx.status = 200;
5146
ctx.body = stream;
5247

53-
result.subscribe((result) => {
54-
stream.write(`data: ${JSON.stringify(result)}\n\n`);
55-
});
48+
result
49+
.subscribe((result) => {
50+
stream.write(`data: ${JSON.stringify(result)}\n\n`);
51+
})
52+
.then(() => {
53+
stream.end();
54+
});
5655
} else {
5756
ctx.request.socket.setTimeout(0);
5857
ctx.req.socket.setNoDelay(true);
@@ -78,13 +77,7 @@ const graphqlHandler = async (ctx: Context) => {
7877
result
7978
.subscribe((result) => {
8079
const chunk = Buffer.from(JSON.stringify(result), "utf8");
81-
const data = [
82-
"",
83-
"Content-Type: application/json; charset=utf-8",
84-
"Content-Length: " + String(chunk.length),
85-
"",
86-
chunk,
87-
];
80+
const data = ["", "Content-Type: application/json; charset=utf-8", "Content-Length: " + String(chunk.length), "", chunk];
8881

8982
if (result.hasNext) {
9083
data.push("---");

0 commit comments

Comments
 (0)