From d10a75c3a9fde41ad7944aeff40ffe4df9946bcd Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Sat, 21 Aug 2021 12:45:14 +0200 Subject: [PATCH] feat: Centralise expected close codes in `CloseCode` enum --- README.md | 23 ++++++----- docs/enums/common.CloseCode.md | 72 ++++++++++++++++++++++++++++++++++ docs/modules/client.md | 7 ++++ docs/modules/common.md | 1 + src/__tests__/client.ts | 19 ++++----- src/__tests__/server.ts | 15 +++---- src/__tests__/use.ts | 19 ++++----- src/client.ts | 19 +++++---- src/common.ts | 18 +++++++++ src/server.ts | 28 +++++++++---- src/use/fastify-websocket.ts | 9 +++-- src/use/uWebSockets.ts | 6 ++- src/use/ws.ts | 13 ++++-- 13 files changed, 193 insertions(+), 56 deletions(-) create mode 100644 docs/enums/common.CloseCode.md diff --git a/README.md b/README.md index 2a67267b..946e45fe 100644 --- a/README.md +++ b/README.md @@ -745,7 +745,7 @@ const client = createClient({ // minimal version of `import { useServer } from 'graphql-ws/lib/use/ws';` import ws from 'ws'; // yarn add ws -import { makeServer } from 'graphql-ws'; +import { makeServer, CloseCode } from 'graphql-ws'; import { schema } from './my-graphql-schema'; // make @@ -779,7 +779,7 @@ wsServer.on('connection', (socket, request) => { } catch (err) { // all errors that could be thrown during the // execution of operations will be caught here - socket.close(4500, err.message); + socket.close(CloseCode.InternalServerError, err.message); } }), }, @@ -802,7 +802,7 @@ wsServer.on('connection', (socket, request) => { import http from 'http'; import ws from 'ws'; // yarn add ws -import { makeServer } from 'graphql-ws'; +import { makeServer, CloseCode } from 'graphql-ws'; import { schema } from './my-graphql-schema'; import { validate } from './my-auth'; @@ -875,7 +875,7 @@ wsServer.on('connection', (socket, request) => { if (err instanceof Forbidden) { // your magic } else { - socket.close(4500, err.message); + socket.close(CloseCode.InternalServerError, err.message); } } }); @@ -897,7 +897,12 @@ wsServer.on('connection', (socket, request) => { ```ts import ws from 'ws'; // yarn add ws -import { makeServer, stringifyMessage, MessageType } from 'graphql-ws'; +import { + makeServer, + CloseCode, + stringifyMessage, + MessageType, +} from 'graphql-ws'; import { schema } from './my-graphql-schema'; // make @@ -946,7 +951,7 @@ wsServer.on('connection', (socket, request) => { } catch (err) { // all errors that could be thrown during the // execution of operations will be caught here - socket.close(4500, err.message); + socket.close(CloseCode.InternalServerError, err.message); } }), // pong received, clear termination timeout @@ -1496,7 +1501,7 @@ useServer( ```typescript // 📺 client -import { createClient } from 'graphql-ws'; +import { createClient, CloseCode } from 'graphql-ws'; import { getCurrentToken, getCurrentTokenExpiresIn, @@ -1540,14 +1545,14 @@ const client = createClient({ // will set the token refresh flag to true tokenExpiryTimeout = setTimeout(() => { if (socket.readyState === WebSocket.OPEN) - socket.close(4403, 'Unauthorized'); + socket.close(CloseCode.Unauthorized, 'Unauthorized'); }, getCurrentTokenExpiresIn()); }, closed: (event) => { // if closed with the `4403: Forbidden` close event // the client or the server is communicating that the token // is no longer valid and should be therefore refreshed - if (event.code === 4403) shouldRefreshToken = true; + if (event.code === CloseCode.Forbidden) shouldRefreshToken = true; }, }, }); diff --git a/docs/enums/common.CloseCode.md b/docs/enums/common.CloseCode.md new file mode 100644 index 00000000..56f3e3e6 --- /dev/null +++ b/docs/enums/common.CloseCode.md @@ -0,0 +1,72 @@ +[graphql-ws](../README.md) / [common](../modules/common.md) / CloseCode + +# Enumeration: CloseCode + +[common](../modules/common.md).CloseCode + +`graphql-ws` expected and standard close codes of the [GraphQL over WebSocket Protocol](/PROTOCOL.md). + +## Table of contents + +### Enumeration members + +- [BadRequest](common.CloseCode.md#badrequest) +- [ConnectionInitialisationTimeout](common.CloseCode.md#connectioninitialisationtimeout) +- [Forbidden](common.CloseCode.md#forbidden) +- [InternalServerError](common.CloseCode.md#internalservererror) +- [SubprotocolNotAcceptable](common.CloseCode.md#subprotocolnotacceptable) +- [SubscriberAlreadyExists](common.CloseCode.md#subscriberalreadyexists) +- [TooManyInitialisationRequests](common.CloseCode.md#toomanyinitialisationrequests) +- [Unauthorized](common.CloseCode.md#unauthorized) + +## Enumeration members + +### BadRequest + +• **BadRequest** = `4400` + +___ + +### ConnectionInitialisationTimeout + +• **ConnectionInitialisationTimeout** = `4408` + +___ + +### Forbidden + +• **Forbidden** = `4403` + +___ + +### InternalServerError + +• **InternalServerError** = `4500` + +___ + +### SubprotocolNotAcceptable + +• **SubprotocolNotAcceptable** = `4406` + +___ + +### SubscriberAlreadyExists + +• **SubscriberAlreadyExists** = `4409` + +Subscriber distinction is very important + +___ + +### TooManyInitialisationRequests + +• **TooManyInitialisationRequests** = `4429` + +___ + +### Unauthorized + +• **Unauthorized** = `4401` + +Tried subscribing before connect ack diff --git a/docs/modules/client.md b/docs/modules/client.md index c2473aee..83a9e01d 100644 --- a/docs/modules/client.md +++ b/docs/modules/client.md @@ -6,6 +6,7 @@ ### References +- [CloseCode](client.md#closecode) - [CompleteMessage](client.md#completemessage) - [ConnectionAckMessage](client.md#connectionackmessage) - [ConnectionInitMessage](client.md#connectioninitmessage) @@ -347,6 +348,12 @@ Creates a disposable GraphQL over WebSocket client. ## Other +### CloseCode + +Re-exports: [CloseCode](../enums/common.CloseCode.md) + +___ + ### CompleteMessage Re-exports: [CompleteMessage](../interfaces/common.CompleteMessage.md) diff --git a/docs/modules/common.md b/docs/modules/common.md index 718c11e3..69ad9ff8 100644 --- a/docs/modules/common.md +++ b/docs/modules/common.md @@ -6,6 +6,7 @@ ### Enumerations +- [CloseCode](../enums/common.CloseCode.md) - [MessageType](../enums/common.MessageType.md) ### Interfaces diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 5c228bb7..ce63c4bf 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -6,6 +6,7 @@ import WebSocket from 'ws'; import { EventEmitter } from 'events'; import { createClient, Client, EventListener } from '../client'; import { + CloseCode, MessageType, parseMessage, stringifyMessage, @@ -212,7 +213,7 @@ it('should close with error message during connecting issues', async () => { await sub.waitForError((err) => { const event = err as CloseEvent; - expect(event.code).toBe(4400); + expect(event.code).toBe(CloseCode.BadRequest); expect(event.reason).toBe('Welcome'); expect(event.wasClean).toBeTruthy(); }); @@ -272,7 +273,7 @@ it('should close the socket if the `connectionParams` rejects or throws', async let sub = tsubscribe(client, { query: '{ getValue }' }); await sub.waitForError((err) => { const event = err as CloseEvent; - expect(event.code).toBe(4400); + expect(event.code).toBe(CloseCode.BadRequest); expect(event.reason).toBe('No auth?'); expect(event.wasClean).toBeTruthy(); }); @@ -287,7 +288,7 @@ it('should close the socket if the `connectionParams` rejects or throws', async sub = tsubscribe(client, { query: '{ getValue }' }); await sub.waitForError((err) => { const event = err as CloseEvent; - expect(event.code).toBe(4400); + expect(event.code).toBe(CloseCode.BadRequest); expect(event.reason).toBe('No auth?'); expect(event.wasClean).toBeTruthy(); }); @@ -1209,13 +1210,13 @@ describe('reconnecting', () => { console.warn = () => { /* hide warnings for test */ }; - await testCloseCode(4406); + await testCloseCode(CloseCode.SubprotocolNotAcceptable); console.warn = warn; - await testCloseCode(4500); - await testCloseCode(4400); - await testCloseCode(4401); - await testCloseCode(4409); - await testCloseCode(4429); + await testCloseCode(CloseCode.InternalServerError); + await testCloseCode(CloseCode.BadRequest); + await testCloseCode(CloseCode.Unauthorized); + await testCloseCode(CloseCode.SubscriberAlreadyExists); + await testCloseCode(CloseCode.TooManyInitialisationRequests); }); it('should report fatal connection problems immediately', async () => { diff --git a/src/__tests__/server.ts b/src/__tests__/server.ts index 0672722f..5ef2b11b 100644 --- a/src/__tests__/server.ts +++ b/src/__tests__/server.ts @@ -11,6 +11,7 @@ import { import { makeServer } from '../server'; import { GRAPHQL_TRANSPORT_WS_PROTOCOL, + CloseCode, MessageType, parseMessage, stringifyMessage, @@ -439,7 +440,7 @@ describe('Connect', () => { ); await client.waitForClose((event) => { - expect(event.code).toBe(4403); + expect(event.code).toBe(CloseCode.Forbidden); expect(event.reason).toBe('Forbidden'); expect(event.wasClean).toBeTruthy(); }); @@ -536,7 +537,7 @@ describe('Connect', () => { await ( await createTClient(url) ).waitForClose((event) => { - expect(event.code).toBe(4408); + expect(event.code).toBe(CloseCode.ConnectionInitialisationTimeout); expect(event.reason).toBe('Connection initialisation timeout'); expect(event.wasClean).toBeTruthy(); }); @@ -590,7 +591,7 @@ describe('Connect', () => { }, 10); await client.waitForClose((event) => { - expect(event.code).toBe(4429); + expect(event.code).toBe(CloseCode.TooManyInitialisationRequests); expect(event.reason).toBe('Too many initialisation requests'); expect(event.wasClean).toBeTruthy(); }); @@ -618,7 +619,7 @@ describe('Connect', () => { ); await client.waitForClose((event) => { - expect(event.code).toBe(4429); + expect(event.code).toBe(CloseCode.TooManyInitialisationRequests); expect(event.reason).toBe('Too many initialisation requests'); expect(event.wasClean).toBeTruthy(); }); @@ -756,7 +757,7 @@ describe('Subscribe', () => { ); await client.waitForClose((event) => { - expect(event.code).toBe(4401); + expect(event.code).toBe(CloseCode.Unauthorized); expect(event.reason).toBe('Unauthorized'); expect(event.wasClean).toBeTruthy(); }); @@ -1401,7 +1402,7 @@ describe('Subscribe', () => { ); await client.waitForClose((event) => { - expect(event.code).toBe(4409); + expect(event.code).toBe(CloseCode.SubscriberAlreadyExists); expect(event.reason).toBe('Subscriber for not-unique already exists'); expect(event.wasClean).toBeTruthy(); }); @@ -1446,7 +1447,7 @@ describe('Subscribe', () => { ); await client.waitForClose((event) => { - expect(event.code).toBe(4409); + expect(event.code).toBe(CloseCode.SubscriberAlreadyExists); expect(event.reason).toBe('Subscriber for not-unique already exists'); expect(event.wasClean).toBeTruthy(); }); diff --git a/src/__tests__/use.ts b/src/__tests__/use.ts index 4cedfe39..9ec50566 100644 --- a/src/__tests__/use.ts +++ b/src/__tests__/use.ts @@ -7,6 +7,7 @@ import { parseMessage, SubscribePayload, GRAPHQL_TRANSPORT_WS_PROTOCOL, + CloseCode, } from '../common'; import { createTClient, @@ -28,14 +29,14 @@ for (const { tServer, startTServer } of tServers) { let client = await createTClient(url, 'notme'); await client.waitForClose((event) => { - expect(event.code).toBe(4406); + expect(event.code).toBe(CloseCode.SubprotocolNotAcceptable); expect(event.reason).toBe('Subprotocol not acceptable'); expect(event.wasClean).toBeTruthy(); }); client = await createTClient(url, ['graphql', 'json']); await client.waitForClose((event) => { - expect(event.code).toBe(4406); + expect(event.code).toBe(CloseCode.SubprotocolNotAcceptable); expect(event.reason).toBe('Subprotocol not acceptable'); expect(event.wasClean).toBeTruthy(); }); @@ -45,7 +46,7 @@ for (const { tServer, startTServer } of tServers) { GRAPHQL_TRANSPORT_WS_PROTOCOL + 'gibberish', ); await client.waitForClose((event) => { - expect(event.code).toBe(4406); + expect(event.code).toBe(CloseCode.SubprotocolNotAcceptable); expect(event.reason).toBe('Subprotocol not acceptable'); expect(event.wasClean).toBeTruthy(); }); @@ -144,7 +145,7 @@ for (const { tServer, startTServer } of tServers) { }), ); await client.waitForClose((event) => { - expect(event.code).toBe(4500); + expect(event.code).toBe(CloseCode.InternalServerError); expect(event.reason).toBe(error.message); expect(event.wasClean).toBeTruthy(); }); @@ -175,7 +176,7 @@ for (const { tServer, startTServer } of tServers) { }); await client.waitForClose((event) => { - expect(event.code).toBe(4500); + expect(event.code).toBe(CloseCode.InternalServerError); expect(event.reason).toBe(error.message); expect(event.wasClean).toBeTruthy(); }); @@ -275,7 +276,7 @@ for (const { tServer, startTServer } of tServers) { }); await client.waitForClose((event) => { - expect(event.code).toBe(4500); + expect(event.code).toBe(CloseCode.InternalServerError); expect(event.reason).toBe('The GraphQL schema is not provided'); expect(event.wasClean).toBeTruthy(); }); @@ -311,7 +312,7 @@ for (const { tServer, startTServer } of tServers) { ); await client.waitForClose((event) => { - expect(event.code).toBe(4500); + expect(event.code).toBe(CloseCode.InternalServerError); expect(event.reason).toBe( 'Invalid return value from onSubscribe hook, expected an array of GraphQLError objects', ); @@ -336,7 +337,7 @@ for (const { tServer, startTServer } of tServers) { ); await client.waitForClose((event) => { - expect(event.code).toBe(4500); + expect(event.code).toBe(CloseCode.InternalServerError); expect(event.reason).toBe(error.message); expect(event.wasClean).toBeTruthy(); }); @@ -353,7 +354,7 @@ for (const { tServer, startTServer } of tServers) { // ws.emit('error', emittedError); // await client.waitForClose((event) => { - // expect(event.code).toBe(4500); // 4500: Internal server error + // expect(event.code).toBe(CloseCode.InternalServerError); // CloseCode.InternalServerError: Internal server error // expect(event.reason).toBe(emittedError.message); // expect(event.wasClean).toBeTruthy(); // because the server reported the error // }); diff --git a/src/client.ts b/src/client.ts index d8079790..d5743d82 100644 --- a/src/client.ts +++ b/src/client.ts @@ -6,6 +6,7 @@ import { GRAPHQL_TRANSPORT_WS_PROTOCOL, + CloseCode, Sink, ID, Disposable, @@ -605,7 +606,7 @@ export function createClient(options: ClientOptions): Client { enqueuePing(); // enqueue ping (noop if disabled) } catch (err) { socket.close( - 4400, + CloseCode.BadRequest, err instanceof Error ? err.message : new Error(err).message, ); } @@ -657,7 +658,7 @@ export function createClient(options: ClientOptions): Client { ]); } catch (err) { socket.close( - 4400, + CloseCode.BadRequest, err instanceof Error ? err.message : new Error(err).message, ); } @@ -710,12 +711,14 @@ export function createClient(options: ClientOptions): Client { if ( isLikeCloseEvent(errOrCloseEvent) && [ - 4500, // Internal server error - 4400, // Bad request - 4401, // Unauthorized (tried subscribing before connect ack) - 4406, // Subprotocol not acceptable - 4409, // Subscriber for already exists (distinction is very important) - 4429, // Too many initialisation requests + CloseCode.InternalServerError, + CloseCode.BadRequest, + CloseCode.Unauthorized, + // CloseCode.Forbidden, might grant access out after retry + CloseCode.SubprotocolNotAcceptable, + // CloseCode.ConnectionInitialisationTimeout, might not time out after retry + CloseCode.SubscriberAlreadyExists, + CloseCode.TooManyInitialisationRequests, ].includes(errOrCloseEvent.code) ) throw errOrCloseEvent; diff --git a/src/common.ts b/src/common.ts index d03d4247..ae08b48d 100644 --- a/src/common.ts +++ b/src/common.ts @@ -20,6 +20,24 @@ import { */ export const GRAPHQL_TRANSPORT_WS_PROTOCOL = 'graphql-transport-ws'; +/** + * `graphql-ws` expected and standard close codes of the [GraphQL over WebSocket Protocol](/PROTOCOL.md). + * + * @category Common + */ +export enum CloseCode { + InternalServerError = 4500, + BadRequest = 4400, + /** Tried subscribing before connect ack */ + Unauthorized = 4401, + Forbidden = 4403, + SubprotocolNotAcceptable = 4406, + ConnectionInitialisationTimeout = 4408, + /** Subscriber distinction is very important */ + SubscriberAlreadyExists = 4409, + TooManyInitialisationRequests = 4429, +} + /** * ID is a string type alias representing * the globally unique ID used for identifying diff --git a/src/server.ts b/src/server.ts index 6d3c6253..d461be9d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -22,6 +22,7 @@ import { } from 'graphql'; import { GRAPHQL_TRANSPORT_WS_PROTOCOL, + CloseCode, ID, Message, MessageType, @@ -544,7 +545,10 @@ export function makeServer(options: ServerOptions): Server { }; if (socket.protocol !== GRAPHQL_TRANSPORT_WS_PROTOCOL) { - socket.close(4406, 'Subprotocol not acceptable'); + socket.close( + CloseCode.SubprotocolNotAcceptable, + 'Subprotocol not acceptable', + ); return async (code, reason) => { /* nothing was set up, just notify the closure */ await onClose?.(ctx, code, reason); @@ -557,7 +561,10 @@ export function makeServer(options: ServerOptions): Server { connectionInitWaitTimeout > 0 && isFinite(connectionInitWaitTimeout) ? setTimeout(() => { if (!ctx.connectionInitReceived) - socket.close(4408, 'Connection initialisation timeout'); + socket.close( + CloseCode.ConnectionInitialisationTimeout, + 'Connection initialisation timeout', + ); }, connectionInitWaitTimeout) : null; @@ -566,12 +573,15 @@ export function makeServer(options: ServerOptions): Server { try { message = parseMessage(data, reviver); } catch (err) { - return socket.close(4400, 'Invalid message received'); + return socket.close(CloseCode.BadRequest, 'Invalid message received'); } switch (message.type) { case MessageType.ConnectionInit: { if (ctx.connectionInitReceived) - return socket.close(4429, 'Too many initialisation requests'); + return socket.close( + CloseCode.TooManyInitialisationRequests, + 'Too many initialisation requests', + ); // @ts-expect-error: I can write ctx.connectionInitReceived = true; @@ -582,7 +592,7 @@ export function makeServer(options: ServerOptions): Server { const permittedOrPayload = await onConnect?.(ctx); if (permittedOrPayload === false) - return socket.close(4403, 'Forbidden'); + return socket.close(CloseCode.Forbidden, 'Forbidden'); await socket.send( stringifyMessage( @@ -623,11 +633,15 @@ export function makeServer(options: ServerOptions): Server { case MessageType.Pong: return await socket.onPong?.(message.payload); case MessageType.Subscribe: { - if (!ctx.acknowledged) return socket.close(4401, 'Unauthorized'); + if (!ctx.acknowledged) + return socket.close(CloseCode.Unauthorized, 'Unauthorized'); const { id, payload } = message; if (id in ctx.subscriptions) - return socket.close(4409, `Subscriber for ${id} already exists`); + return socket.close( + CloseCode.SubscriberAlreadyExists, + `Subscriber for ${id} already exists`, + ); // if this turns out to be a streaming operation, the subscription value // will change to an `AsyncIterable`, otherwise it will stay as is diff --git a/src/use/fastify-websocket.ts b/src/use/fastify-websocket.ts index 2885dba2..508bf1a8 100644 --- a/src/use/fastify-websocket.ts +++ b/src/use/fastify-websocket.ts @@ -1,7 +1,7 @@ import type { FastifyRequest } from 'fastify'; import type * as fastifyWebsocket from 'fastify-websocket'; import { makeServer, ServerOptions } from '../server'; -import { GRAPHQL_TRANSPORT_WS_PROTOCOL } from '../common'; +import { GRAPHQL_TRANSPORT_WS_PROTOCOL, CloseCode } from '../common'; /** * The extra that will be put in the `Context`. @@ -47,7 +47,10 @@ export function makeHandler< const { socket } = connection; socket.on('error', (err) => - socket.close(4500, isProd ? 'Internal server error' : err.message), + socket.close( + CloseCode.InternalServerError, + isProd ? 'Internal server error' : err.message, + ), ); // keep alive through ping-pong messages @@ -89,7 +92,7 @@ export function makeHandler< await cb(String(event)); } catch (err) { socket.close( - 4500, + CloseCode.InternalServerError, isProd ? 'Internal server error' : err.message, ); } diff --git a/src/use/uWebSockets.ts b/src/use/uWebSockets.ts index 2e19aee1..b5f08b70 100644 --- a/src/use/uWebSockets.ts +++ b/src/use/uWebSockets.ts @@ -1,6 +1,7 @@ import type * as uWS from 'uWebSockets.js'; import type http from 'http'; import { makeServer, ServerOptions } from '../server'; +import { CloseCode } from '../common'; /** * The extra that will be put in the `Context`. @@ -188,7 +189,10 @@ export function makeBehavior< try { await client.handleMessage(Buffer.from(message).toString()); } catch (err) { - socket.end(4500, isProd ? 'Internal server error' : err.message); + socket.end( + CloseCode.InternalServerError, + isProd ? 'Internal server error' : err.message, + ); } }, close(...args) { diff --git a/src/use/ws.ts b/src/use/ws.ts index 085cba0c..fbe77f42 100644 --- a/src/use/ws.ts +++ b/src/use/ws.ts @@ -1,7 +1,11 @@ import type * as http from 'http'; import type * as ws from 'ws'; import { makeServer, ServerOptions } from '../server'; -import { GRAPHQL_TRANSPORT_WS_PROTOCOL, Disposable } from '../common'; +import { + GRAPHQL_TRANSPORT_WS_PROTOCOL, + CloseCode, + Disposable, +} from '../common'; // for nicer documentation type WebSocket = typeof ws.prototype; @@ -54,7 +58,10 @@ export function useServer< // report server errors by erroring out all clients with the same error for (const client of ws.clients) { try { - client.close(4500, isProd ? 'Internal server error' : err.message); + client.close( + CloseCode.InternalServerError, + isProd ? 'Internal server error' : err.message, + ); } catch (err) { firstErr = firstErr ?? err; } @@ -103,7 +110,7 @@ export function useServer< await cb(String(event)); } catch (err) { socket.close( - 4500, + CloseCode.InternalServerError, isProd ? 'Internal server error' : err.message, ); }