Skip to content

Commit

Permalink
fix: Use 4500 close code for internal server errors (1011 is an i…
Browse files Browse the repository at this point in the history
…nternal WebSocket close code)
  • Loading branch information
enisdenjo committed Aug 21, 2021
1 parent df85281 commit 3c0316d
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 18 deletions.
6 changes: 3 additions & 3 deletions README.md
Expand Up @@ -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(1011, err.message);
socket.close(4500, err.message);
}
}),
},
Expand Down Expand Up @@ -875,7 +875,7 @@ wsServer.on('connection', (socket, request) => {
if (err instanceof Forbidden) {
// your magic
} else {
socket.close(1011, err.message);
socket.close(4500, err.message);
}
}
});
Expand Down Expand Up @@ -946,7 +946,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(1011, err.message);
socket.close(4500, err.message);
}
}),
// pong received, clear termination timeout
Expand Down
2 changes: 1 addition & 1 deletion docs/interfaces/client.clientoptions.md
Expand Up @@ -166,7 +166,7 @@ ___
How many times should the client try to reconnect on abnormal socket closure before it errors out?

The library classifies the following close events as fatal:
- `1011: Internal Error`
- `4500: Internal server error`
- `4400: Bad Request`
- `4401: Unauthorized` _tried subscribing before connect ack_
- `4406: Subprotocol not acceptable`
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/client.ts
Expand Up @@ -1211,7 +1211,7 @@ describe('reconnecting', () => {
};
await testCloseCode(4406);
console.warn = warn;
await testCloseCode(1011);
await testCloseCode(4500);
await testCloseCode(4400);
await testCloseCode(4401);
await testCloseCode(4409);
Expand Down
12 changes: 6 additions & 6 deletions src/__tests__/use.ts
Expand Up @@ -144,7 +144,7 @@ for (const { tServer, startTServer } of tServers) {
}),
);
await client.waitForClose((event) => {
expect(event.code).toBe(1011);
expect(event.code).toBe(4500);
expect(event.reason).toBe(error.message);
expect(event.wasClean).toBeTruthy();
});
Expand Down Expand Up @@ -175,7 +175,7 @@ for (const { tServer, startTServer } of tServers) {
});

await client.waitForClose((event) => {
expect(event.code).toBe(1011);
expect(event.code).toBe(4500);
expect(event.reason).toBe(error.message);
expect(event.wasClean).toBeTruthy();
});
Expand Down Expand Up @@ -275,7 +275,7 @@ for (const { tServer, startTServer } of tServers) {
});

await client.waitForClose((event) => {
expect(event.code).toBe(1011);
expect(event.code).toBe(4500);
expect(event.reason).toBe('The GraphQL schema is not provided');
expect(event.wasClean).toBeTruthy();
});
Expand Down Expand Up @@ -311,7 +311,7 @@ for (const { tServer, startTServer } of tServers) {
);

await client.waitForClose((event) => {
expect(event.code).toBe(1011);
expect(event.code).toBe(4500);
expect(event.reason).toBe(
'Invalid return value from onSubscribe hook, expected an array of GraphQLError objects',
);
Expand All @@ -336,7 +336,7 @@ for (const { tServer, startTServer } of tServers) {
);

await client.waitForClose((event) => {
expect(event.code).toBe(1011);
expect(event.code).toBe(4500);
expect(event.reason).toBe(error.message);
expect(event.wasClean).toBeTruthy();
});
Expand All @@ -353,7 +353,7 @@ for (const { tServer, startTServer } of tServers) {
// ws.emit('error', emittedError);

// await client.waitForClose((event) => {
// expect(event.code).toBe(1011); // 1011: Internal Error
// expect(event.code).toBe(4500); // 4500: Internal server error
// expect(event.reason).toBe(emittedError.message);
// expect(event.wasClean).toBeTruthy(); // because the server reported the error
// });
Expand Down
4 changes: 2 additions & 2 deletions src/client.ts
Expand Up @@ -314,7 +314,7 @@ export interface ClientOptions {
* How many times should the client try to reconnect on abnormal socket closure before it errors out?
*
* The library classifies the following close events as fatal:
* - `1011: Internal Error`
* - `4500: Internal server error`
* - `4400: Bad Request`
* - `4401: Unauthorized` _tried subscribing before connect ack_
* - `4406: Subprotocol not acceptable`
Expand Down Expand Up @@ -710,7 +710,7 @@ export function createClient(options: ClientOptions): Client {
if (
isLikeCloseEvent(errOrCloseEvent) &&
[
1011, // Internal Error
4500, // Internal server error
4400, // Bad Request
4401, // Unauthorized (tried subscribing before connect ack)
4406, // Subprotocol not acceptable
Expand Down
7 changes: 5 additions & 2 deletions src/use/fastify-websocket.ts
Expand Up @@ -47,7 +47,7 @@ export function makeHandler<
const { socket } = connection;

socket.on('error', (err) =>
socket.close(1011, isProd ? 'Internal Error' : err.message),
socket.close(4500, isProd ? 'Internal server error' : err.message),
);

// keep alive through ping-pong messages
Expand Down Expand Up @@ -88,7 +88,10 @@ export function makeHandler<
try {
await cb(String(event));
} catch (err) {
socket.close(1011, isProd ? 'Internal Error' : err.message);
socket.close(
4500,
isProd ? 'Internal server error' : err.message,
);
}
}),
},
Expand Down
2 changes: 1 addition & 1 deletion src/use/uWebSockets.ts
Expand Up @@ -188,7 +188,7 @@ export function makeBehavior<
try {
await client.handleMessage(Buffer.from(message).toString());
} catch (err) {
socket.end(1011, isProd ? 'Internal Error' : err.message);
socket.end(4500, isProd ? 'Internal server error' : err.message);
}
},
close(...args) {
Expand Down
7 changes: 5 additions & 2 deletions src/use/ws.ts
Expand Up @@ -54,7 +54,7 @@ export function useServer<
// report server errors by erroring out all clients with the same error
for (const client of ws.clients) {
try {
client.close(1011, isProd ? 'Internal Error' : err.message);
client.close(4500, isProd ? 'Internal server error' : err.message);
} catch (err) {
firstErr = firstErr ?? err;
}
Expand Down Expand Up @@ -102,7 +102,10 @@ export function useServer<
try {
await cb(String(event));
} catch (err) {
socket.close(1011, isProd ? 'Internal Error' : err.message);
socket.close(
4500,
isProd ? 'Internal server error' : err.message,
);
}
}),
},
Expand Down

0 comments on commit 3c0316d

Please sign in to comment.