Skip to content

Commit 46340ef

Browse files
dyedwiperdyedwiper
authored
BC-9633 Fix error handling and close codes (#66)
1 parent 5fa0ab6 commit 46340ef

File tree

5 files changed

+33
-28
lines changed

5 files changed

+33
-28
lines changed

‎src/infra/authorization/authorization.service.spec.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createMock, DeepMocked } from '@golevelup/ts-jest';
22
import { Test, TestingModule } from '@nestjs/testing';
33
import { HttpRequest } from 'uWebSockets.js';
4+
import { WebSocketCloseCode } from '../../shared/type/websocket-close-code.js';
45
import { Logger } from '../logger/index.js';
56
import { AuthorizationApi, AuthorizedReponse } from './authorization-api-client/index.js';
67
import { AuthorizationService } from './authorization.service.js';
@@ -82,7 +83,7 @@ describe(AuthorizationService.name, () => {
8283

8384
const expectedResult = {
8485
error: {
85-
code: 4401,
86+
code: WebSocketCloseCode.Unauthorized,
8687
reason: 'Unauthorized',
8788
},
8889
hasWriteAccess: false,
@@ -108,7 +109,7 @@ describe(AuthorizationService.name, () => {
108109

109110
const expectedResult = {
110111
error: {
111-
code: 4500,
112+
code: WebSocketCloseCode.InternalError,
112113
reason: 'RoomId not found',
113114
},
114115
hasWriteAccess: false,
@@ -133,8 +134,8 @@ describe(AuthorizationService.name, () => {
133134
const { req } = setupRequest('roomId', 'other=ABC');
134135
const expectedResult = {
135136
error: {
136-
code: 4500,
137-
reason: 'JWT token not found',
137+
code: WebSocketCloseCode.Unauthorized,
138+
reason: 'JWT not found',
138139
},
139140
hasWriteAccess: false,
140141
room: null,
@@ -162,7 +163,7 @@ describe(AuthorizationService.name, () => {
162163

163164
const expectedResult = {
164165
error: {
165-
code: 4500,
166+
code: WebSocketCloseCode.InternalError,
166167
reason: 'testError',
167168
},
168169
hasWriteAccess: false,

‎src/infra/authorization/authorization.service.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Injectable } from '@nestjs/common';
22
import { HttpRequest } from 'uWebSockets.js';
3+
import { WebSocketCloseCode } from '../../shared/type/websocket-close-code.js';
34
import { Logger } from '../logger/index.js';
45
import {
56
AuthorizationApi,
@@ -28,7 +29,11 @@ export class AuthorizationService {
2829

2930
response = await this.fetchAuthorization(room, token);
3031
} catch (error) {
31-
response = this.createErrorResponsePayload(4500, error.message);
32+
if (error.message === 'JWT not found') {
33+
response = this.createErrorResponsePayload(WebSocketCloseCode.Unauthorized, 'JWT not found');
34+
} else {
35+
response = this.createErrorResponsePayload(WebSocketCloseCode.InternalError, error.message);
36+
}
3237
}
3338

3439
return response;
@@ -45,13 +50,13 @@ export class AuthorizationService {
4550
}
4651

4752
private getToken(req: HttpRequest): string {
48-
const jwtToken = this.getCookie(req, 'jwt');
53+
const token = this.getCookie(req, 'jwt');
4954

50-
if (!jwtToken) {
51-
throw new Error('JWT token not found');
55+
if (!token) {
56+
throw new Error('JWT not found');
5257
}
5358

54-
return jwtToken;
59+
return token;
5560
}
5661

5762
private async fetchAuthorization(room: string, token: string): Promise<ResponsePayload> {
@@ -80,7 +85,7 @@ export class AuthorizationService {
8085

8186
const { isAuthorized, userId } = response;
8287
if (!isAuthorized) {
83-
return this.createErrorResponsePayload(4401, 'Unauthorized');
88+
return this.createErrorResponsePayload(WebSocketCloseCode.Unauthorized, 'Unauthorized');
8489
}
8590

8691
return this.createResponsePayload(room, userId);

‎src/modules/server/api/test/websocket.api.spec.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { WebsocketProvider } from 'y-websocket';
88
import { Doc, encodeStateAsUpdateV2 } from 'yjs';
99
import { ResponsePayloadBuilder } from '../../../../infra//authorization/response.builder.js';
1010
import { AuthorizationService } from '../../../../infra/authorization/index.js';
11+
import { WebSocketCloseCode } from '../../../../shared/type/websocket-close-code.js';
1112
import { ServerModule } from '../../server.module.js';
1213
import { TldrawServerConfig } from '../../tldraw-server.config.js';
1314

@@ -184,7 +185,7 @@ describe('Websocket Api Test', () => {
184185
const randomString = Math.random().toString(36).substring(7);
185186
const room = randomString;
186187

187-
const errorResponse = ResponsePayloadBuilder.buildWithError(4401, 'Unauthorized');
188+
const errorResponse = ResponsePayloadBuilder.buildWithError(WebSocketCloseCode.Unauthorized, 'Unauthorized');
188189
authorizationService.hasPermission.mockResolvedValueOnce(errorResponse);
189190

190191
const { ydoc: client1Doc, provider } = createWsClient(room);
@@ -209,7 +210,7 @@ describe('Websocket Api Test', () => {
209210
// @ts-ignore
210211
expect(error.reason).toBe('Unauthorized');
211212
// @ts-ignore
212-
expect(error.code).toBe(4401);
213+
expect(error.code).toBe(WebSocketCloseCode.Unauthorized);
213214

214215
provider.awareness.destroy();
215216
provider.destroy();
@@ -246,7 +247,7 @@ describe('Websocket Api Test', () => {
246247
// @ts-ignore
247248
expect(error.reason).toBe('Missing room or userid');
248249
// @ts-ignore
249-
expect(error.code).toBe(1008);
250+
expect(error.code).toBe(WebSocketCloseCode.InternalError);
250251

251252
provider.awareness.destroy();
252253
provider.destroy();
@@ -281,7 +282,7 @@ describe('Websocket Api Test', () => {
281282
// @ts-ignore
282283
expect(error.reason).toBe('Internal Server Error');
283284
// @ts-ignore
284-
expect(error.code).toBe(1011);
285+
expect(error.code).toBe(WebSocketCloseCode.InternalError);
285286
});
286287
});
287288
});

‎src/modules/server/api/websocket.gateway.ts‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Logger } from '../../../infra/logger/index.js';
1212
import { MetricsService } from '../../../infra/metrics/index.js';
1313
import { RedisAdapter } from '../../../infra/redis/index.js';
1414
import { YRedisClient, YRedisDoc, YRedisService, YRedisUser, YRedisUserFactory } from '../../../infra/y-redis/index.js';
15+
import { WebSocketCloseCode } from '../../../shared/type/websocket-close-code.js';
1516
import { REDIS_FOR_SUBSCRIBE_OF_DELETION, UWS } from '../server.const.js';
1617
import { TldrawServerConfig } from '../tldraw-server.config.js';
1718

@@ -21,14 +22,6 @@ interface RequestHeaderInfos {
2122
headerWsProtocol: string;
2223
}
2324

24-
// https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code
25-
enum WebSocketErrorCodes {
26-
InternalError = 1011,
27-
PolicyViolation = 1008,
28-
TldrawPolicyViolation = 4401,
29-
TldrawInternalError = 4500,
30-
}
31-
3225
@Injectable()
3326
export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
3427
public constructor(
@@ -120,7 +113,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
120113
}
121114

122115
if (user.room === null || user.userid === null) {
123-
ws.end(WebSocketErrorCodes.PolicyViolation, 'Missing room or userid');
116+
ws.end(WebSocketCloseCode.InternalError, 'Missing room or userid');
124117

125118
return;
126119
}
@@ -149,7 +142,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
149142
this.yRedisService.ensureLatestContentSubscription(yRedisDoc, user);
150143
} catch (error) {
151144
this.logger.warning(error);
152-
ws.end(WebSocketErrorCodes.InternalError, 'Internal Server Error');
145+
ws.end(WebSocketCloseCode.InternalError, 'Internal Server Error');
153146
}
154147
}
155148

@@ -176,7 +169,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
176169
const user = ws.getUserData();
177170

178171
if (!user.hasWriteAccess || !user.room) {
179-
ws.end(WebSocketErrorCodes.TldrawPolicyViolation, 'User has no write access or room is missing');
172+
ws.end(WebSocketCloseCode.Unauthorized, 'User has no write access or room is missing');
180173

181174
return;
182175
}
@@ -188,7 +181,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
188181
}
189182
} catch (error) {
190183
this.logger.warning(error);
191-
ws.end(WebSocketErrorCodes.InternalError);
184+
ws.end(WebSocketCloseCode.InternalError);
192185
}
193186
}
194187

@@ -207,7 +200,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
207200
MetricsService.openConnectionsGauge.dec();
208201
} catch (error) {
209202
this.logger.warning(error);
210-
ws.end(WebSocketErrorCodes.InternalError);
203+
ws.end(WebSocketCloseCode.InternalError);
211204
}
212205
}
213206

‎src/shared/type/websocket-close-code.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code
2+
export enum WebSocketCloseCode {
3+
InternalError = 1011,
4+
Unauthorized = 4401,
5+
}

0 commit comments

Comments
 (0)