From 723f3f611cf7306f037564441a728d6fab7e4430 Mon Sep 17 00:00:00 2001
From: Philip Molares <philip.molares@udo.edu>
Date: Sun, 22 Oct 2023 21:33:34 +0200
Subject: [PATCH] feat(realtime): add disconnect reason
The frontend now doesn't try to reconnect, when the disconnection happened because of a lack of permissions
Signed-off-by: Philip Molares <philip.molares@udo.edu>
---
.../backend-websocket-adapter.spec.ts | 34 +++++++++++++------
.../websocket/backend-websocket-adapter.ts | 18 +++++++---
.../realtime/websocket/websocket.gateway.ts | 5 ++-
.../message-transporters/disconnect_reason.ts | 9 +++++
commons/src/message-transporters/index.ts | 1 +
.../message-transporter.ts | 30 ++++++++++------
commons/src/message-transporters/message.ts | 8 ++++-
.../message-transporters/transport-adapter.ts | 3 +-
.../yjs/frontend-websocket-adapter.spec.ts | 19 ++++++++---
.../hooks/yjs/frontend-websocket-adapter.ts | 10 +++---
.../hooks/yjs/use-realtime-connection.ts | 20 +++++++----
11 files changed, 111 insertions(+), 46 deletions(-)
create mode 100644 commons/src/message-transporters/disconnect_reason.ts
diff --git a/backend/src/realtime/websocket/backend-websocket-adapter.spec.ts b/backend/src/realtime/websocket/backend-websocket-adapter.spec.ts
index 4bae2cf4c..0bda5122d 100644
--- a/backend/src/realtime/websocket/backend-websocket-adapter.spec.ts
+++ b/backend/src/realtime/websocket/backend-websocket-adapter.spec.ts
@@ -3,9 +3,14 @@
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
-import { ConnectionState, Message, MessageType } from '@hedgedoc/commons';
+import {
+ ConnectionState,
+ DisconnectReason,
+ Message,
+ MessageType,
+} from '@hedgedoc/commons';
import { Mock } from 'ts-mockery';
-import WebSocket, { MessageEvent } from 'ws';
+import WebSocket, { CloseEvent, MessageEvent } from 'ws';
import { BackendWebsocketAdapter } from './backend-websocket-adapter';
@@ -29,17 +34,26 @@ describe('backend websocket adapter', () => {
});
it('can bind and unbind the close event', () => {
- const handler = jest.fn();
+ const handler = jest.fn((reason?: DisconnectReason) => console.log(reason));
+
+ let modifiedHandler: (event: CloseEvent) => void = jest.fn();
+ jest
+ .spyOn(mockedSocket, 'addEventListener')
+ .mockImplementation((event, handler_) => {
+ modifiedHandler = handler_;
+ });
+
const unbind = sut.bindOnCloseEvent(handler);
- expect(mockedSocket.addEventListener).toHaveBeenCalledWith(
- 'close',
- handler,
+
+ modifiedHandler(
+ Mock.of<CloseEvent>({ code: DisconnectReason.USER_NOT_PERMITTED }),
);
+ expect(handler).toHaveBeenCalledTimes(1);
+ expect(handler).toHaveBeenCalledWith(DisconnectReason.USER_NOT_PERMITTED);
+
unbind();
- expect(mockedSocket.removeEventListener).toHaveBeenCalledWith(
- 'close',
- handler,
- );
+
+ expect(mockedSocket.removeEventListener).toHaveBeenCalled();
});
it('can bind and unbind the connect event', () => {
diff --git a/backend/src/realtime/websocket/backend-websocket-adapter.ts b/backend/src/realtime/websocket/backend-websocket-adapter.ts
index c59a8893a..4040a50fb 100644
--- a/backend/src/realtime/websocket/backend-websocket-adapter.ts
+++ b/backend/src/realtime/websocket/backend-websocket-adapter.ts
@@ -3,9 +3,14 @@
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
-import { ConnectionState, Message, MessageType } from '@hedgedoc/commons';
+import {
+ ConnectionState,
+ DisconnectReason,
+ Message,
+ MessageType,
+} from '@hedgedoc/commons';
import type { TransportAdapter } from '@hedgedoc/commons';
-import WebSocket, { MessageEvent } from 'ws';
+import WebSocket, { CloseEvent, MessageEvent } from 'ws';
/**
* Implements a transport adapter that communicates using a nodejs socket.
@@ -13,10 +18,13 @@ import WebSocket, { MessageEvent } from 'ws';
export class BackendWebsocketAdapter implements TransportAdapter {
constructor(private socket: WebSocket) {}
- bindOnCloseEvent(handler: () => void): () => void {
- this.socket.addEventListener('close', handler);
+ bindOnCloseEvent(handler: (reason?: DisconnectReason) => void): () => void {
+ function callback(event: CloseEvent): void {
+ handler(event.code);
+ }
+ this.socket.addEventListener('close', callback);
return () => {
- this.socket.removeEventListener('close', handler);
+ this.socket.removeEventListener('close', callback);
};
}
diff --git a/backend/src/realtime/websocket/websocket.gateway.ts b/backend/src/realtime/websocket/websocket.gateway.ts
index 75facc4dd..ad0466d46 100644
--- a/backend/src/realtime/websocket/websocket.gateway.ts
+++ b/backend/src/realtime/websocket/websocket.gateway.ts
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
import {
+ DisconnectReason,
MessageTransporter,
NotePermissions,
userCanEdit,
@@ -66,13 +67,11 @@ export class WebsocketGateway implements OnGatewayConnection {
note,
);
if (notePermission < NotePermission.READ) {
- //TODO: [mrdrogdrog] inform client about reason of disconnect.
- // (https://github.com/hedgedoc/hedgedoc/issues/5034)
this.logger.log(
`Access denied to note '${note.id}' for user '${username}'`,
'handleConnection',
);
- clientSocket.close();
+ clientSocket.close(DisconnectReason.USER_NOT_PERMITTED);
return;
}
diff --git a/commons/src/message-transporters/disconnect_reason.ts b/commons/src/message-transporters/disconnect_reason.ts
new file mode 100644
index 000000000..070bd4842
--- /dev/null
+++ b/commons/src/message-transporters/disconnect_reason.ts
@@ -0,0 +1,9 @@
+/*
+ * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+export enum DisconnectReason {
+ USER_NOT_PERMITTED = 4000
+}
diff --git a/commons/src/message-transporters/index.ts b/commons/src/message-transporters/index.ts
index 20e27e223..229af5a4a 100644
--- a/commons/src/message-transporters/index.ts
+++ b/commons/src/message-transporters/index.ts
@@ -9,3 +9,4 @@ export * from './message-transporter.js'
export * from './realtime-user.js'
export * from './transport-adapter.js'
export * from './mocked-backend-transport-adapter.js'
+export * from './disconnect_reason.js'
diff --git a/commons/src/message-transporters/message-transporter.ts b/commons/src/message-transporters/message-transporter.ts
index c070da270..c8d3fc180 100644
--- a/commons/src/message-transporters/message-transporter.ts
+++ b/commons/src/message-transporters/message-transporter.ts
@@ -1,17 +1,25 @@
/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
-import { Message, MessagePayloads, MessageType } from './message.js'
+import {
+ ConnectionStateEvent,
+ Message,
+ MessagePayloads,
+ MessageType
+} from './message.js'
import { TransportAdapter } from './transport-adapter.js'
import { EventEmitter2, Listener } from 'eventemitter2'
+import { DisconnectReason } from './disconnect_reason.js'
-export type MessageEvents = MessageType | 'connected' | 'disconnected' | 'ready'
+export type AllEvents = MessageType | ConnectionStateEvent
type MessageEventPayloadMap = {
- [E in MessageEvents]: E extends keyof MessagePayloads
+ [E in AllEvents]: E extends keyof MessagePayloads
? (message: Message<E>) => void
+ : E extends ConnectionStateEvent.DISCONNECTED
+ ? (reason?: DisconnectReason) => void
: () => void
}
@@ -157,14 +165,14 @@ export class MessageTransporter extends EventEmitter2<MessageEventPayloadMap> {
}
}
- private bindWebsocketEvents(websocket: TransportAdapter) {
- this.destroyOnErrorEventHandler = websocket.bindOnErrorEvent(
+ private bindWebsocketEvents(transportAdapter: TransportAdapter) {
+ this.destroyOnErrorEventHandler = transportAdapter.bindOnErrorEvent(
this.onDisconnecting.bind(this)
)
- this.destroyOnCloseEventHandler = websocket.bindOnCloseEvent(
+ this.destroyOnCloseEventHandler = transportAdapter.bindOnCloseEvent(
this.onDisconnecting.bind(this)
)
- this.destroyOnMessageEventHandler = websocket.bindOnMessageEvent(
+ this.destroyOnMessageEventHandler = transportAdapter.bindOnMessageEvent(
this.receiveMessage.bind(this)
)
}
@@ -172,10 +180,10 @@ export class MessageTransporter extends EventEmitter2<MessageEventPayloadMap> {
protected onConnected(): void {
this.destroyOnConnectedEventHandler?.()
this.destroyOnConnectedEventHandler = undefined
- this.emit('connected')
+ this.emit(ConnectionStateEvent.CONNECTED)
}
- protected onDisconnecting(): void {
+ protected onDisconnecting(reason?: DisconnectReason): void {
if (this.transportAdapter === undefined) {
return
}
@@ -184,7 +192,7 @@ export class MessageTransporter extends EventEmitter2<MessageEventPayloadMap> {
this.thisSideReady = false
this.otherSideReady = false
this.transportAdapter = undefined
- this.emit('disconnected')
+ this.emit(ConnectionStateEvent.DISCONNECTED, reason)
}
/**
diff --git a/commons/src/message-transporters/message.ts b/commons/src/message-transporters/message.ts
index 76fd22c47..f45e33c06 100644
--- a/commons/src/message-transporters/message.ts
+++ b/commons/src/message-transporters/message.ts
@@ -1,5 +1,5 @@
/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
@@ -22,6 +22,12 @@ export enum MessageType {
READY_ANSWER = 'READY_ANSWER'
}
+export enum ConnectionStateEvent {
+ READY = 'ready',
+ CONNECTED = 'connected',
+ DISCONNECTED = 'disconnected'
+}
+
export interface MessagePayloads {
[MessageType.NOTE_CONTENT_STATE_REQUEST]: number[]
[MessageType.NOTE_CONTENT_UPDATE]: number[]
diff --git a/commons/src/message-transporters/transport-adapter.ts b/commons/src/message-transporters/transport-adapter.ts
index 0a14f37cd..5d1a0680b 100644
--- a/commons/src/message-transporters/transport-adapter.ts
+++ b/commons/src/message-transporters/transport-adapter.ts
@@ -5,6 +5,7 @@
*/
import { ConnectionState } from './message-transporter.js'
import { Message, MessageType } from './message.js'
+import { DisconnectReason } from './disconnect_reason.js'
/**
* Defines methods that must be implemented to send and receive messages using an {@link AdapterMessageTransporter}.
@@ -18,7 +19,7 @@ export interface TransportAdapter {
bindOnErrorEvent(handler: () => void): () => void
- bindOnCloseEvent(handler: () => void): () => void
+ bindOnCloseEvent(handler: (reason?: DisconnectReason) => void): () => void
disconnect(): void
diff --git a/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.spec.ts b/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.spec.ts
index 57c747567..9e6ba52bc 100644
--- a/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.spec.ts
+++ b/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.spec.ts
@@ -5,7 +5,7 @@
*/
import { FrontendWebsocketAdapter } from './frontend-websocket-adapter'
import type { Message } from '@hedgedoc/commons'
-import { ConnectionState, MessageType } from '@hedgedoc/commons'
+import { ConnectionState, DisconnectReason, MessageType } from '@hedgedoc/commons'
import { Mock } from 'ts-mockery'
describe('frontend websocket', () => {
@@ -34,11 +34,22 @@ describe('frontend websocket', () => {
it('can bind and unbind the close event', () => {
mockSocket()
- const handler = jest.fn()
+ const handler = jest.fn((reason?: DisconnectReason) => console.log(reason))
+
+ let modifiedHandler: (event: CloseEvent) => void = jest.fn()
+ jest.spyOn(mockedSocket, 'addEventListener').mockImplementation((event, handler_) => {
+ modifiedHandler = handler_
+ })
+
const unbind = adapter.bindOnCloseEvent(handler)
- expect(addEventListenerSpy).toHaveBeenCalledWith('close', handler)
+
+ modifiedHandler(Mock.of<CloseEvent>({ code: DisconnectReason.USER_NOT_PERMITTED }))
+ expect(handler).toHaveBeenCalledTimes(1)
+ expect(handler).toHaveBeenCalledWith(DisconnectReason.USER_NOT_PERMITTED)
+
unbind()
- expect(removeEventListenerSpy).toHaveBeenCalledWith('close', handler)
+
+ expect(removeEventListenerSpy).toHaveBeenCalled()
})
it('can bind and unbind the connect event', () => {
diff --git a/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.ts b/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.ts
index a3deb3ba2..6e7eda5b5 100644
--- a/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.ts
+++ b/frontend/src/components/editor-page/editor-pane/hooks/yjs/frontend-websocket-adapter.ts
@@ -3,9 +3,8 @@
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
-import type { TransportAdapter } from '@hedgedoc/commons'
+import type { DisconnectReason, Message, MessageType, TransportAdapter } from '@hedgedoc/commons'
import { ConnectionState } from '@hedgedoc/commons'
-import type { Message, MessageType } from '@hedgedoc/commons'
/**
* Implements a transport adapter that communicates using a browser websocket.
@@ -13,10 +12,11 @@ import type { Message, MessageType } from '@hedgedoc/commons'
export class FrontendWebsocketAdapter implements TransportAdapter {
constructor(private socket: WebSocket) {}
- bindOnCloseEvent(handler: () => void): () => void {
- this.socket.addEventListener('close', handler)
+ bindOnCloseEvent(handler: (reason?: DisconnectReason) => void): () => void {
+ const callback = (event: CloseEvent): void => handler(event.code)
+ this.socket.addEventListener('close', callback)
return () => {
- this.socket.removeEventListener('close', handler)
+ this.socket.removeEventListener('close', callback)
}
}
diff --git a/frontend/src/components/editor-page/editor-pane/hooks/yjs/use-realtime-connection.ts b/frontend/src/components/editor-page/editor-pane/hooks/yjs/use-realtime-connection.ts
index 79813c2a2..a461b5ac0 100644
--- a/frontend/src/components/editor-page/editor-pane/hooks/yjs/use-realtime-connection.ts
+++ b/frontend/src/components/editor-page/editor-pane/hooks/yjs/use-realtime-connection.ts
@@ -1,5 +1,5 @@
/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
@@ -10,7 +10,7 @@ import { Logger } from '../../../../../utils/logger'
import { isMockMode } from '../../../../../utils/test-modes'
import { FrontendWebsocketAdapter } from './frontend-websocket-adapter'
import { useWebsocketUrl } from './use-websocket-url'
-import { MessageTransporter, MockedBackendTransportAdapter } from '@hedgedoc/commons'
+import { DisconnectReason, MessageTransporter, MockedBackendTransportAdapter } from '@hedgedoc/commons'
import type { Listener } from 'eventemitter2'
import { useCallback, useEffect, useMemo, useRef } from 'react'
@@ -28,6 +28,7 @@ export const useRealtimeConnection = (): MessageTransporter => {
const messageTransporter = useMemo(() => new MessageTransporter(), [])
const reconnectCount = useRef(0)
+ const disconnectReason = useRef<DisconnectReason | undefined>(undefined)
const establishWebsocketConnection = useCallback(() => {
if (isMockMode) {
logger.debug('Creating Loopback connection...')
@@ -57,7 +58,7 @@ export const useRealtimeConnection = (): MessageTransporter => {
const isConnected = useApplicationState((state) => state.realtimeStatus.isConnected)
useEffect(() => {
- if (isConnected || reconnectCount.current > 0) {
+ if (isConnected || reconnectCount.current > 0 || disconnectReason.current === DisconnectReason.USER_NOT_PERMITTED) {
return
}
establishWebsocketConnection()
@@ -86,9 +87,16 @@ export const useRealtimeConnection = (): MessageTransporter => {
useEffect(() => {
const connectedListener = messageTransporter.doAsSoonAsReady(() => setRealtimeConnectionState(true))
- const disconnectedListener = messageTransporter.on('disconnected', () => setRealtimeConnectionState(false), {
- objectify: true
- }) as Listener
+ const disconnectedListener = messageTransporter.on(
+ 'disconnected',
+ (reason?: DisconnectReason) => {
+ disconnectReason.current = reason
+ setRealtimeConnectionState(false)
+ },
+ {
+ objectify: true
+ }
+ ) as Listener
return () => {
connectedListener.off()