Skip to content

Commit

Permalink
fix: broken GraphiQL implementation + GraphiQL puppeteer tests (#83)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
n1ru4l committed Oct 24, 2021
1 parent 1f5eeda commit f4399bb
Show file tree
Hide file tree
Showing 14 changed files with 9,703 additions and 10,001 deletions.
5 changes: 5 additions & 0 deletions .changeset/poor-spiders-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"graphql-helix": patch
---

terminate SSE HTTP connection after stream ended emitting values
5 changes: 5 additions & 0 deletions .changeset/wet-rules-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"graphql-helix": patch
---

fix broken multi part response and SSE response fetching in GraphiQL
1 change: 1 addition & 0 deletions packages/core/lib/send-result/node-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export async function sendPushResult(
// @ts-expect-error - Different Signature between ServerResponse and Http2ServerResponse but still compatible.
rawResponse.write(`data: ${JSON.stringify(transformResult(result))}\n\n`);
});
rawResponse.end();
}

export async function sendResult(
Expand Down
1 change: 1 addition & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"jest": "27.2.1",
"lint-staged": "11.1.2",
"move-file-cli": "3.0.0",
"puppeteer": "10.4.0",
"replacestream": "4.0.3",
"ts-jest": "27.0.5",
"ts-node": "10.2.1",
Expand Down
48 changes: 2 additions & 46 deletions packages/core/test/implementations/express.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import express, { RequestHandler } from "express";
import { getGraphQLParameters, processRequest, renderGraphiQL, shouldRenderGraphiQL } from "../../lib";
import { getGraphQLParameters, processRequest, renderGraphiQL, sendResult, shouldRenderGraphiQL } from "../../lib";
import { schema } from "../schema";

const graphqlMiddleware: RequestHandler = async (req, res) => {
Expand All @@ -18,51 +18,7 @@ const graphqlMiddleware: RequestHandler = async (req, res) => {
schema,
});

if (result.type === "RESPONSE") {
result.headers.forEach(({ name, value }) => res.setHeader(name, value));
res.status(result.status);
res.json(result.payload);
} else if (result.type === "PUSH") {
res.writeHead(200, {
"Content-Type": "text/event-stream",
Connection: "keep-alive",
"Cache-Control": "no-cache",
});

req.on("close", () => {
result.unsubscribe();
});

await result.subscribe((result) => {
res.write(`data: ${JSON.stringify(result)}\n\n`);
});
} else {
res.writeHead(200, {
Connection: "keep-alive",
"Content-Type": 'multipart/mixed; boundary="-"',
"Transfer-Encoding": "chunked",
});

req.on("close", () => {
result.unsubscribe();
});

res.write("---");

await result.subscribe((result) => {
const chunk = Buffer.from(JSON.stringify(result), "utf8");
const data = ["", "Content-Type: application/json; charset=utf-8", "Content-Length: " + String(chunk.length), "", chunk];

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

res.write(data.join("\r\n"));
});

res.write("\r\n-----\r\n");
res.end();
}
await sendResult(result, res);
};

const graphiqlMiddleware: RequestHandler = async (_req, res) => {
Expand Down
61 changes: 4 additions & 57 deletions packages/core/test/implementations/fastify.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import fastify, { RouteHandlerMethod } from "fastify";
import {
getGraphQLParameters,
processRequest,
renderGraphiQL,
shouldRenderGraphiQL,
} from "../../lib";
import { getGraphQLParameters, processRequest, renderGraphiQL, sendResult, shouldRenderGraphiQL } from "../../lib";
import { schema } from "../schema";

const graphqlHandler: RouteHandlerMethod = async (req, res) => {
Expand All @@ -23,57 +18,9 @@ const graphqlHandler: RouteHandlerMethod = async (req, res) => {
schema,
});

if (result.type === "RESPONSE") {
result.headers.forEach(({ name, value }) => res.header(name, value));
res.status(result.status);
res.send(result.payload);
} else if (result.type === "PUSH") {
res.raw.writeHead(200, {
"Content-Type": "text/event-stream",
Connection: "keep-alive",
"Cache-Control": "no-cache",
});

req.raw.on("close", () => {
result.unsubscribe();
});

await result.subscribe((result) => {
res.raw.write(`data: ${JSON.stringify(result)}\n\n`);
});
} else {
res.raw.writeHead(200, {
Connection: "keep-alive",
"Content-Type": 'multipart/mixed; boundary="-"',
"Transfer-Encoding": "chunked",
});

req.raw.on("close", () => {
result.unsubscribe();
});

res.raw.write("---");

await result.subscribe((result) => {
const chunk = Buffer.from(JSON.stringify(result), "utf8");
const data = [
"",
"Content-Type: application/json; charset=utf-8",
"Content-Length: " + String(chunk.length),
"",
chunk,
];

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

res.raw.write(data.join("\r\n"));
});

res.raw.write("\r\n-----\r\n");
res.raw.end();
}
await sendResult(result, res.raw);
// Tell fastify a response was sent
res.sent = true;
};

const graphiqlHandler: RouteHandlerMethod = async (_req, res) => {
Expand Down
64 changes: 5 additions & 59 deletions packages/core/test/implementations/fastify_async.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
import fastify, { RouteHandlerMethod } from "fastify";
import { parse as graphqlParse } from "graphql";
import {
getGraphQLParameters,
processRequest,
renderGraphiQL,
shouldRenderGraphiQL,
} from "../../lib";
import { getGraphQLParameters, processRequest, renderGraphiQL, sendResult, shouldRenderGraphiQL } from "../../lib";
import { schema } from "../schema";

const sleep = (time: number) =>
new Promise<void>((resolve) => setTimeout(resolve, time));
const sleep = (time: number) => new Promise<void>((resolve) => setTimeout(resolve, time));

const graphqlHandler: RouteHandlerMethod = async (req, res) => {
const request = {
Expand All @@ -31,57 +25,9 @@ const graphqlHandler: RouteHandlerMethod = async (req, res) => {
},
});

if (result.type === "RESPONSE") {
result.headers.forEach(({ name, value }) => res.header(name, value));
res.status(result.status);
res.send(result.payload);
} else if (result.type === "PUSH") {
res.raw.writeHead(200, {
"Content-Type": "text/event-stream",
Connection: "keep-alive",
"Cache-Control": "no-cache",
});

req.raw.on("close", () => {
result.unsubscribe();
});

await result.subscribe((result) => {
res.raw.write(`data: ${JSON.stringify(result)}\n\n`);
});
} else {
res.raw.writeHead(200, {
Connection: "keep-alive",
"Content-Type": 'multipart/mixed; boundary="-"',
"Transfer-Encoding": "chunked",
});

req.raw.on("close", () => {
result.unsubscribe();
});

res.raw.write("---");

await result.subscribe((result) => {
const chunk = Buffer.from(JSON.stringify(result), "utf8");
const data = [
"",
"Content-Type: application/json; charset=utf-8",
"Content-Length: " + String(chunk.length),
"",
chunk,
];

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

res.raw.write(data.join("\r\n"));
});

res.raw.write("\r\n-----\r\n");
res.raw.end();
}
await sendResult(result, res.raw);
// Tell fastify a response was sent
res.sent = true;
};

const graphiqlHandler: RouteHandlerMethod = async (_req, res) => {
Expand Down
25 changes: 9 additions & 16 deletions packages/core/test/implementations/koa.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import Koa, { Context } from "koa";
import bodyParser from "koa-bodyparser";
import { PassThrough } from "stream";
import {
getGraphQLParameters,
processRequest,
renderGraphiQL,
shouldRenderGraphiQL,
} from "../../lib";
import { getGraphQLParameters, processRequest, renderGraphiQL, shouldRenderGraphiQL } from "../../lib";
import { schema } from "../schema";

const graphqlHandler = async (ctx: Context) => {
Expand Down Expand Up @@ -50,9 +45,13 @@ const graphqlHandler = async (ctx: Context) => {
ctx.status = 200;
ctx.body = stream;

result.subscribe((result) => {
stream.write(`data: ${JSON.stringify(result)}\n\n`);
});
result
.subscribe((result) => {
stream.write(`data: ${JSON.stringify(result)}\n\n`);
})
.then(() => {
stream.end();
});
} else {
ctx.request.socket.setTimeout(0);
ctx.req.socket.setNoDelay(true);
Expand All @@ -78,13 +77,7 @@ const graphqlHandler = async (ctx: Context) => {
result
.subscribe((result) => {
const chunk = Buffer.from(JSON.stringify(result), "utf8");
const data = [
"",
"Content-Type: application/json; charset=utf-8",
"Content-Length: " + String(chunk.length),
"",
chunk,
];
const data = ["", "Content-Type: application/json; charset=utf-8", "Content-Length: " + String(chunk.length), "", chunk];

if (result.hasNext) {
data.push("---");
Expand Down
Loading

0 comments on commit f4399bb

Please sign in to comment.