Skip to content

Commit

Permalink
feat: Centralise expected close codes in CloseCode enum
Browse files Browse the repository at this point in the history
enisdenjo committed Aug 21, 2021

Verified

This commit was signed with the committer’s verified signature.
enisdenjo Denis Badurina
1 parent dedcc6e commit d10a75c
Showing 13 changed files with 193 additions and 56 deletions.
23 changes: 14 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
@@ -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;
},
},
});
72 changes: 72 additions & 0 deletions docs/enums/common.CloseCode.md
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions docs/modules/client.md
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions docs/modules/common.md
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@

### Enumerations

- [CloseCode](../enums/common.CloseCode.md)
- [MessageType](../enums/common.MessageType.md)

### Interfaces
19 changes: 10 additions & 9 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
15 changes: 8 additions & 7 deletions src/__tests__/server.ts
Original file line number Diff line number Diff line change
@@ -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();
});
19 changes: 10 additions & 9 deletions src/__tests__/use.ts
Original file line number Diff line number Diff line change
@@ -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
// });
19 changes: 11 additions & 8 deletions src/client.ts
Original file line number Diff line number Diff line change
@@ -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 <id> 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;
18 changes: 18 additions & 0 deletions src/common.ts
Original file line number Diff line number Diff line change
@@ -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
28 changes: 21 additions & 7 deletions src/server.ts
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@ import {
} from 'graphql';
import {
GRAPHQL_TRANSPORT_WS_PROTOCOL,
CloseCode,
ID,
Message,
MessageType,
@@ -544,7 +545,10 @@ export function makeServer<E = unknown>(options: ServerOptions<E>): Server<E> {
};

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<E = unknown>(options: ServerOptions<E>): Server<E> {
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<E = unknown>(options: ServerOptions<E>): Server<E> {
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<E = unknown>(options: ServerOptions<E>): Server<E> {

const permittedOrPayload = await onConnect?.(ctx);
if (permittedOrPayload === false)
return socket.close(4403, 'Forbidden');
return socket.close(CloseCode.Forbidden, 'Forbidden');

await socket.send(
stringifyMessage<MessageType.ConnectionAck>(
@@ -623,11 +633,15 @@ export function makeServer<E = unknown>(options: ServerOptions<E>): Server<E> {
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
9 changes: 6 additions & 3 deletions src/use/fastify-websocket.ts
Original file line number Diff line number Diff line change
@@ -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,
);
}
6 changes: 5 additions & 1 deletion src/use/uWebSockets.ts
Original file line number Diff line number Diff line change
@@ -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) {
13 changes: 10 additions & 3 deletions src/use/ws.ts
Original file line number Diff line number Diff line change
@@ -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,
);
}

0 comments on commit d10a75c

Please sign in to comment.