Repository: mercurius-js/mercurius
Pull request #987 (2023-04-26 07:37:12)

Update to @fastify/websocket v8
Updates to `@fastify/websocket` v8 to address a leak and crash in certain condition. This requires a minimum version of Fastify of v4.16.0 (just released), hence the major update.

mcollina: cc @simoneb @codeflyer @marco-ippolito @jonnydgreen wdyt?

This fixes a horrible crash in Platformatic, so I'm keen on shipping this sooner rather than later.
simoneb: I'm afraid I'm missing some context but it looks like this is part of a group of upgrades to various packages that need to go together (I see the major in fastify-ws, a minor followed by various patches in fastify, fastify-json-stringify and compiler, ...). There's an overarching plan I'm missing, but I guess we're good as I wouldn't be in any better position than you are to validate this change.
mcollina: There is no grand plan, just this bug to be fixed https://github.com/fastify/fastify-websocket/issues/249. That required a new hook in Fastify and and a semver-major change jn fastify-websocket.

The change in fast-json-stringify was 100% unrelated as well as all the problems in v4.16.3.

Patch:
diff --git a/index.js b/index.js
index 67e85f3..171b7fa 100644
--- a/index.js
+++ b/index.js
@@ -471,7 +471,7 @@ const plugin = fp(async function (app, opts) {
       document = cached.document
     }
 
-    if (reply && reply.request.raw.method === 'GET') {
+    if (reply && reply.request.raw.method === 'GET' && !reply.request.ws) {
       // let's validate we cannot do mutations here
       const operationAST = getOperationAST(document, operationName)
       if (operationAST.operation !== 'query') {
diff --git a/package.json b/package.json
index 4cf1f5e..9d4b2b7 100644
--- a/package.json
+++ b/package.json
@@ -42,7 +42,7 @@
     "autocannon": "^7.3.0",
     "concurrently": "^8.0.1",
     "docsify-cli": "^4.4.3",
-    "fastify": "^4.0.0",
+    "fastify": "^4.16.0",
     "graphql-ws": "^5.11.2",
     "pre-commit": "^1.2.2",
     "proxyquire": "^2.1.3",
@@ -58,7 +58,7 @@
   "dependencies": {
     "@fastify/error": "^3.0.0",
     "@fastify/static": "^6.0.0",
-    "@fastify/websocket": "^7.0.0",
+    "@fastify/websocket": "^8.0.0",
     "fastify-plugin": "^4.2.0",
     "graphql": "^16.0.0",
     "graphql-jit": "^0.8.0",


Test patch:
diff --git a/tap-snapshots/test/routes.js.test.cjs b/tap-snapshots/test/routes.js.test.cjs
index 91a38f5..90cce74 100644
--- a/tap-snapshots/test/routes.js.test.cjs
+++ b/tap-snapshots/test/routes.js.test.cjs
@@ -27,6 +27,17 @@ exports['test/routes.js TAP GET return 200 on resolver error > must match snapsh
 }
 `
 
+exports['test/routes.js TAP HTTP mutation with GET errors > must match snapshot 1'] = `
+{
+  "data": null,
+  "errors": [
+    {
+      "message": "Operation cannot be performed via a GET request"
+    }
+  ]
+}
+`
+
 exports['test/routes.js TAP POST return 200 on resolver error > must match snapshot 1'] = `
 {
   "data": {
@@ -71,17 +82,6 @@ window.GRAPHQL_ENDPOINT = '/app/graphql';
 window.GRAPHIQL_PLUGIN_LIST = []
 `
 
-exports['test/routes.js TAP mutation with GET errors > must match snapshot 1'] = `
-{
-  "data": null,
-  "errors": [
-    {
-      "message": "Operation cannot be performed via a GET request"
-    }
-  ]
-}
-`
-
 exports['test/routes.js TAP server should return 200 on graphql errors (if field can be null) > must match snapshot 1'] = `
 {
   "data": {
diff --git a/test/routes.js b/test/routes.js
index b6366ba..73a6a16 100644
--- a/test/routes.js
+++ b/test/routes.js
@@ -756,7 +756,7 @@ test('mutation with POST application/graphql', async (t) => {
   t.equal(msg, 'hello world')
 })
 
-test('mutation with GET errors', async (t) => {
+test('HTTP mutation with GET errors', async (t) => {
   const app = Fastify()
   const schema = `
     type Mutation {
@@ -789,6 +789,61 @@ test('mutation with GET errors', async (t) => {
   t.matchSnapshot(JSON.stringify(JSON.parse(res.body), null, 2))
 })
 
+test('websocket mutation with GET allowed', async (t) => {
+  const app = Fastify()
+
+  // Simulate fastify-websocket logic
+  app.addHook('onRequest', (request, reply, done) => {
+    request.ws = true
+    done()
+  })
+
+  const schema = `
+    type Mutation {
+      setMessage(message: String): String
+    }
+    type Query {
+      getMessage: String
+    }
+  `
+
+  let msg = 'hello'
+  const resolvers = {
+    setMessage: async ({ message }) => {
+      msg = message
+      return message
+    },
+    async getMessage () {
+      return msg
+    }
+  }
+
+  app.register(GQL, {
+    schema,
+    resolvers
+  })
+
+  const query = querystring.stringify({
+    query: 'mutation { setMessage(message: "hello world") }'
+  })
+
+  const res = await app.inject({
+    headers: {
+      connection: 'upgrade',
+      upgrade: 'websocket'
+    },
+    method: 'GET',
+    url: '/graphql?' + query
+  })
+
+  t.same(JSON.parse(res.body), {
+    data: {
+      setMessage: 'hello world'
+    }
+  })
+  t.equal(msg, 'hello world')
+})
+
 test('POST should support null variables', async (t) => {
   const app = Fastify()
   const schema = `