sync: don't auto-create missing rooms; instead show a 'not found' error (#3551)

This PR removes the auto-creation of new rooms via just hitting a URL
endpoint on dotcom. We now serve up (effectively) a 404. The tricky bit
of this is that the 404 status is passed through the websocket. As such,
there's quite a bit of plumbing to be able to bubble up a not-found
event from the server→ client.

I'll copy the comment that I put in the code on how it passes through
the code:
1. From here (`getInitialRoomState`), we send back a `room_not_found` to
`TLDrawDurableObject`.
2. In `TLDrawDurableObject`, we accept and
and then immediately close the client. This lets us send a
`TLCloseEventCode.NOT_FOUND`
  closeCode down to the client.
3. `joinExistingRoom` which handles the websocket upgrade is not
affected.
Again, we accept the connection, it's just that we immediately close
right after.
4. In `ClientWebSocketAdapter`, `ws.onclose` is called, and that calls
`_handleDisconnect`.
5. `_handleDisconnect` sets the status to 'error' and calls the
`onStatusChange` callback.
6. On dotcom in `useRemoteSyncClient`, we have `socket.onStatusChange`
callback
where we set `TLIncompatibilityReason.RoomNotFound` and close the client
+ socket.
7. Finally on the dotcom app we use `StoreErrorScreen` to display an
appropriate msg.

Phew!

<img width="1344" alt="Screenshot 2024-04-23 at 14 13 17"
src="https://github.com/tldraw/tldraw/assets/469604/8f05861f-378a-421c-b53b-a96bd3fa12b8">


### Change Type

<!-- ❗ Please select a 'Scope' label ❗️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!-- ❗ Please select a 'Type' label ❗️ -->

- [ ] `bugfix` — Bug fix
- [x] `feature` — New feature
- [ ] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know


### Test Plan

1. Test `/r/random-slug` doesn't create a new room, nor does `/r`
2. Make sure `/r/existing-slug` still works.

### Release Notes

- Rooms: `/r/:roomId` no longer creates a new room. Instead, if the room
doesn't exist we bubble up a `room_not_found` error.

apps/dotcom-worker/src/lib/TLDrawDurableObject.ts

@@ -4,7 +4,9 @@
 import { SupabaseClient } from '@supabase/supabase-js'
 import { ROOM_OPEN_MODE, type RoomOpenMode } from '@tldraw/dotcom-shared'
 import {
+	DBLoadResultType,
 	RoomSnapshot,
+	TLCloseEventCode,
 	TLServer,
 	TLServerEvent,
 	TLSyncRoom,
@@ -241,9 +243,10 @@ export class TLDrawDurableObject extends TLServer {
 		const { 0: clientWebSocket, 1: serverWebSocket } = new WebSocketPair()
 
 		// Handle the connection (see TLServer)
+		let connectionResult: DBLoadResultType
 		try {
 			// block concurrency while initializing the room if that needs to happen
-			await this.controller.blockConcurrencyWhile(() =>
+			connectionResult = await this.controller.blockConcurrencyWhile(() =>
 				this.handleConnection({
 					socket: serverWebSocket as any,
 					persistenceKey: this.documentInfo.slug!,
@@ -268,6 +271,12 @@ export class TLDrawDurableObject extends TLServer {
 			this.schedulePersist()
 		})
 
+		if (connectionResult === 'room_not_found') {
+			// If the room is not found, we need to accept and then immediately close the connection
+			// with our custom close code.
+			serverWebSocket.close(TLCloseEventCode.NOT_FOUND, 'Room not found')
+		}
+
 		return new Response(null, { status: 101, webSocket: clientWebSocket })
 	}
 

apps/dotcom/src/components/ErrorPage/ErrorPage.tsx

@@ -1,4 +1,5 @@
 import { Link } from 'react-router-dom'
+import { isInIframe } from '../../utils/iFrame'
 
 export function ErrorPage({
 	icon,
@@ -19,9 +20,7 @@ export function ErrorPage({
 					<p>{messages.para1}</p>
 					{messages.para2 && <p>{messages.para2}</p>}
 				</div>
-				<Link to={'/'}>
-					<a>Take me home.</a>
-				</Link>
+				<Link to={'/'}>{isInIframe() ? 'Open tldraw.' : 'Back to tldraw.'}</Link>
 			</div>
 		</div>
 	)

apps/dotcom/src/components/StoreErrorScreen.tsx

@@ -1,9 +1,11 @@
 import { TLIncompatibilityReason } from '@tldraw/tlsync'
-import { ErrorScreen, exhaustiveSwitchError } from 'tldraw'
+import { exhaustiveSwitchError } from 'tldraw'
 import { RemoteSyncError } from '../utils/remote-sync/remote-sync'
+import { ErrorPage } from './ErrorPage/ErrorPage'
 
 export function StoreErrorScreen({ error }: { error: Error }) {
-	let message = 'Could not connect to server.'
+	let header = 'Could not connect to server.'
+	let message = ''
 
 	if (error instanceof RemoteSyncError) {
 		switch (error.reason) {
@@ -26,14 +28,15 @@ export function StoreErrorScreen({ error }: { error: Error }) {
 					'Your changes were rejected by the server. Please reload the page. If the problem persists contact the system administrator.'
 				break
 			}
+			case TLIncompatibilityReason.RoomNotFound: {
+				header = 'Room not found'
+				message = 'The room you are trying to connect to does not exist.'
+				break
+			}
 			default:
 				exhaustiveSwitchError(error.reason)
 		}
 	}
 
-	return (
-		<div className="tldraw__editor tl-container">
-			<ErrorScreen>{message}</ErrorScreen>
-		</div>
-	)
+	return <ErrorPage icon messages={{ header, para1: message }} />
 }

apps/dotcom/src/hooks/useRemoteSyncClient.ts

@@ -1,4 +1,10 @@
-import { TLSyncClient, schema } from '@tldraw/tlsync'
+import {
+	TLCloseEventCode,
+	TLIncompatibilityReason,
+	TLPersistentClientSocketStatus,
+	TLSyncClient,
+	schema,
+} from '@tldraw/tlsync'
 import { useEffect, useState } from 'react'
 import {
 	TAB_ID,
@@ -55,6 +61,16 @@ export function useRemoteSyncClient(opts: UseSyncClientConfig): RemoteTLStoreWit
 			return withParams.toString()
 		})
 
+		socket.onStatusChange((val: TLPersistentClientSocketStatus, closeCode?: number) => {
+			if (val === 'error' && closeCode === TLCloseEventCode.NOT_FOUND) {
+				trackAnalyticsEvent(MULTIPLAYER_EVENT_NAME, { name: 'room-not-found', roomId })
+				setState({ error: new RemoteSyncError(TLIncompatibilityReason.RoomNotFound) })
+				client.close()
+				socket.close()
+				return
+			}
+		})
+
 		let didCancel = false
 
 		const client = new TLSyncClient({

apps/dotcom/src/utils/remote-sync/ClientWebSocketAdapter.test.ts

@@ -155,20 +155,33 @@ describe(ClientWebSocketAdapter, () => {
 	it('signals status changes', async () => {
 		const onStatusChange = jest.fn()
 		adapter.onStatusChange(onStatusChange)
+
 		await waitFor(() => adapter._ws?.readyState === WebSocket.OPEN)
 		expect(onStatusChange).toHaveBeenCalledWith('online')
 		connectedServerSocket.terminate()
 		await waitFor(() => adapter._ws?.readyState === WebSocket.CLOSED)
-		expect(onStatusChange).toHaveBeenCalledWith('offline')
+		expect(onStatusChange).toHaveBeenCalledWith('offline', 1006)
+
 		await waitFor(() => adapter._ws?.readyState === WebSocket.OPEN)
 		expect(onStatusChange).toHaveBeenCalledWith('online')
 		connectedServerSocket.terminate()
 		await waitFor(() => adapter._ws?.readyState === WebSocket.CLOSED)
-		expect(onStatusChange).toHaveBeenCalledWith('offline')
+		expect(onStatusChange).toHaveBeenCalledWith('offline', 1006)
+
 		await waitFor(() => adapter._ws?.readyState === WebSocket.OPEN)
 		expect(onStatusChange).toHaveBeenCalledWith('online')
 		adapter._ws?.onerror?.({} as any)
-		expect(onStatusChange).toHaveBeenCalledWith('error')
+		expect(onStatusChange).toHaveBeenCalledWith('error', undefined)
+	})
+
+	it('signals the correct closeCode when a room is not found', async () => {
+		const onStatusChange = jest.fn()
+		adapter.onStatusChange(onStatusChange)
+		await waitFor(() => adapter._ws?.readyState === WebSocket.OPEN)
+
+		adapter._ws!.onclose?.({ code: 4099 } as any)
+
+		expect(onStatusChange).toHaveBeenCalledWith('error', 4099)
 	})
 
 	it('signals status changes while restarting', async () => {
@@ -181,7 +194,7 @@ describe(ClientWebSocketAdapter, () => {
 
 		await waitFor(() => onStatusChange.mock.calls.length === 2)
 
-		expect(onStatusChange).toHaveBeenCalledWith('offline')
+		expect(onStatusChange).toHaveBeenCalledWith('offline', undefined)
 		expect(onStatusChange).toHaveBeenCalledWith('online')
 	})
 })

apps/dotcom/src/utils/remote-sync/ClientWebSocketAdapter.ts

@@ -1,5 +1,6 @@
 import {
 	chunk,
+	TLCloseEventCode,
 	TLPersistentClientSocket,
 	TLPersistentClientSocketStatus,
 	TLSocketClientSentEvent,
@@ -68,15 +69,20 @@ export class ClientWebSocketAdapter implements TLPersistentClientSocket<TLRecord
 		this._reconnectManager.connected()
 	}
 
-	private _handleDisconnect(reason: 'closed' | 'error' | 'manual') {
+	private _handleDisconnect(reason: 'closed' | 'error' | 'manual', closeCode?: number) {
 		debug('handleDisconnect', {
 			currentStatus: this.connectionStatus,
+			closeCode,
 			reason,
 		})
 
 		let newStatus: 'offline' | 'error'
 		switch (reason) {
 			case 'closed':
+				if (closeCode === TLCloseEventCode.NOT_FOUND) {
+					newStatus = 'error'
+					break
+				}
 				newStatus = 'offline'
 				break
 			case 'error':
@@ -94,7 +100,7 @@ export class ClientWebSocketAdapter implements TLPersistentClientSocket<TLRecord
 			!(newStatus === 'error' && this.connectionStatus === 'offline')
 		) {
 			this._connectionStatus.set(newStatus)
-			this.statusListeners.forEach((cb) => cb(newStatus))
+			this.statusListeners.forEach((cb) => cb(newStatus, closeCode))
 		}
 
 		this._reconnectManager.disconnected()
@@ -120,10 +126,10 @@ export class ClientWebSocketAdapter implements TLPersistentClientSocket<TLRecord
 			)
 			this._handleConnect()
 		}
-		ws.onclose = () => {
+		ws.onclose = (event: CloseEvent) => {
 			debug('ws.onclose')
 			if (this._ws === ws) {
-				this._handleDisconnect('closed')
+				this._handleDisconnect('closed', event.code)
 			} else {
 				debug('ignoring onclose for an orphaned socket')
 			}
@@ -194,8 +200,10 @@ export class ClientWebSocketAdapter implements TLPersistentClientSocket<TLRecord
 		}
 	}
 
-	private statusListeners = new Set<(status: TLPersistentClientSocketStatus) => void>()
-	onStatusChange(cb: (val: TLPersistentClientSocketStatus) => void) {
+	private statusListeners = new Set<
+		(status: TLPersistentClientSocketStatus, closeCode?: number) => void
+	>()
+	onStatusChange(cb: (val: TLPersistentClientSocketStatus, closeCode?: number) => void) {
 		assert(!this.isDisposed, 'Tried to add status listener on a disposed socket')
 
 		this.statusListeners.add(cb)

apps/dotcom/styles/core.css

@@ -183,6 +183,7 @@ a {
 	font-weight: 500;
 	color: var(--text-color-2);
 	padding: 12px 4px;
+	text-decoration: underline;
 }
 
 /* ------------------ Board history ----------------- */

packages/tlsync/src/index.ts

@@ -1,5 +1,11 @@
-export { TLServer, type DBLoadResult, type TLServerEvent } from './lib/TLServer'
 export {
+	TLServer,
+	type DBLoadResult,
+	type DBLoadResultType,
+	type TLServerEvent,
+} from './lib/TLServer'
+export {
+	TLCloseEventCode,
 	TLSyncClient,
 	type TLPersistentClientSocket,
 	type TLPersistentClientSocketStatus,

packages/tlsync/src/lib/TLServer.ts

@@ -6,7 +6,7 @@ import { JsonChunkAssembler } from './chunk'
 import { schema } from './schema'
 import { RoomState } from './server-types'
 
-type LoadKind = 'new' | 'reopen' | 'open'
+type LoadKind = 'reopen' | 'open' | 'room_not_found'
 export type DBLoadResult =
 	| {
 			type: 'error'
@@ -19,6 +19,7 @@ export type DBLoadResult =
 	| {
 			type: 'room_not_found'
 	  }
+export type DBLoadResultType = DBLoadResult['type']
 
 export type TLServerEvent =
 	| {
@@ -54,10 +55,10 @@ export type TLServerEvent =
 export abstract class TLServer {
 	schema = schema
 
-	async getInitialRoomState(persistenceKey: string): Promise<[RoomState, LoadKind]> {
+	async getInitialRoomState(persistenceKey: string): Promise<[RoomState | undefined, LoadKind]> {
 		let roomState = this.getRoomForPersistenceKey(persistenceKey)
 
-		let roomOpenKind = 'open' as 'open' | 'reopen' | 'new'
+		let roomOpenKind: LoadKind = 'open'
 
 		// If no room exists for the id, create one
 		if (roomState === undefined) {
@@ -78,14 +79,22 @@ export abstract class TLServer {
 				}
 			}
 
-			// If we still don't have a room, create a new one
+			// If we still don't have a room, throw an error.
 			if (roomState === undefined) {
-				roomOpenKind = 'new'
-
-				roomState = {
-					persistenceKey,
-					room: new TLSyncRoom(this.schema),
-				}
+				// This is how it bubbles down to the client:
+				// 1.) From here, we send back a `room_not_found` to TLDrawDurableObject.
+				// 2.) In TLDrawDurableObject, we accept and then immediately close the client.
+				//   This lets us send a TLCloseEventCode.NOT_FOUND closeCode down to the client.
+				// 3.) joinExistingRoom which handles the websocket upgrade is not affected.
+				//   Again, we accept the connection, it's just that we immediately close right after.
+				// 4.) In ClientWebSocketAdapter, ws.onclose is called, and that calls _handleDisconnect.
+				// 5.) _handleDisconnect sets the status to 'error' and calls the onStatusChange callback.
+				// 6.) On the dotcom app in useRemoteSyncClient, we have socket.onStatusChange callback
+				//   where we set TLIncompatibilityReason.RoomNotFound and close the client + socket.
+				// 7.) Finally on the dotcom app we use StoreErrorScreen to display an appropriate msg.
+				//
+				// Phew!
+				return [roomState, 'room_not_found']
 			}
 
 			const thisRoom = roomState.room
@@ -138,9 +147,13 @@ export abstract class TLServer {
 		persistenceKey: string
 		sessionKey: string
 		storeId: string
-	}) => {
+	}): Promise<DBLoadResultType> => {
 		const clientId = nanoid()
+
 		const [roomState, roomOpenKind] = await this.getInitialRoomState(persistenceKey)
+		if (roomOpenKind === 'room_not_found' || !roomState) {
+			return 'room_not_found'
+		}
 
 		roomState.room.handleNewSession(
 			sessionKey,
@@ -156,7 +169,7 @@ export abstract class TLServer {
 			})
 		)
 
-		if (roomOpenKind === 'new' || roomOpenKind === 'reopen') {
+		if (roomOpenKind === 'reopen') {
 			// Record that the room is now active
 			this.logEvent({ type: 'room', roomId: persistenceKey, name: 'room_start' })
 
@@ -164,7 +177,7 @@ export abstract class TLServer {
 			this.logEvent({
 				type: 'client',
 				roomId: persistenceKey,
-				name: roomOpenKind === 'new' ? 'room_create' : 'room_reopen',
+				name: 'room_reopen',
 				clientId,
 				instanceId: sessionKey,
 				localClientId: storeId,
@@ -237,6 +250,8 @@ export abstract class TLServer {
 		socket.addEventListener('message', handleMessageFromClient)
 		socket.addEventListener('close', handleCloseOrErrorFromClient)
 		socket.addEventListener('error', handleCloseOrErrorFromClient)
+
+		return 'room_found'
 	}
 
 	/**

packages/tlsync/src/lib/TLSyncClient.ts

@@ -24,6 +24,17 @@ import './requestAnimationFrame.polyfill'
 
 type SubscribingFn<T> = (cb: (val: T) => void) => () => void
 
+/**
+ * These are our private codes to be sent from server->client.
+ * They are in the private range of the websocket code range.
+ * See: https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code
+ *
+ * @public
+ */
+export const TLCloseEventCode = {
+	NOT_FOUND: 4099,
+} as const
+
 /** @public */
 export type TLPersistentClientSocketStatus = 'online' | 'offline' | 'error'
 /**
@@ -236,6 +247,7 @@ export class TLSyncClient<R extends UnknownRecord, S extends Store<R> = Store<R>
 				})
 			)
 		}
+
 		// if the socket is already online before this client was instantiated
 		// then we should send a connect message right away
 		if (this.socket.connectionStatus === 'online') {

packages/tlsync/src/lib/protocol.ts

@@ -10,6 +10,7 @@ export const TLIncompatibilityReason = {
 	ServerTooOld: 'serverTooOld',
 	InvalidRecord: 'invalidRecord',
 	InvalidOperation: 'invalidOperation',
+	RoomNotFound: 'roomNotFound',
 } as const
 
 /** @public */

packages/tlsync/src/test/TLServer.test.ts

@@ -1,7 +1,17 @@
-import { TLRecord, createTLStore, defaultShapeUtils } from 'tldraw'
+import {
+	DocumentRecordType,
+	PageRecordType,
+	RecordId,
+	TLDocument,
+	TLRecord,
+	ZERO_INDEX_KEY,
+	createTLStore,
+	defaultShapeUtils,
+} from 'tldraw'
 import { type WebSocket } from 'ws'
 import { RoomSessionState } from '../lib/RoomSession'
 import { DBLoadResult, TLServer } from '../lib/TLServer'
+import { RoomSnapshot } from '../lib/TLSyncRoom'
 import { chunk } from '../lib/chunk'
 import { RecordOpType } from '../lib/diff'
 import { TLSYNC_PROTOCOL_VERSION, TLSocketClientSentEvent } from '../lib/protocol'
@@ -17,6 +27,16 @@ const PORT = 23473
 
 const disposables: (() => void)[] = []
 
+const records = [
+	DocumentRecordType.create({ id: 'document:document' as RecordId<TLDocument> }),
+	PageRecordType.create({ index: ZERO_INDEX_KEY, name: 'page 2' }),
+]
+const makeSnapshot = (records: TLRecord[], others: Partial<RoomSnapshot> = {}) => ({
+	documents: records.map((r) => ({ state: r, lastChangedClock: 0 })),
+	clock: 0,
+	...others,
+})
+
 class TLServerTestImpl extends TLServer {
 	wsServer = new ws.Server({ port: PORT })
 	async close() {
@@ -54,7 +74,7 @@ class TLServerTestImpl extends TLServer {
 		}
 	}
 	override async loadFromDatabase?(_roomId: string): Promise<DBLoadResult> {
-		return { type: 'room_not_found' }
+		return { type: 'room_found', snapshot: makeSnapshot(records) }
 	}
 	override async persistToDatabase?(_roomId: string): Promise<void> {
 		return
@@ -84,15 +104,20 @@ beforeEach(async () => {
 	sockets = await server.createSocketPair()
 	expect(sockets.client.readyState).toBe(ws.OPEN)
 	expect(sockets.server.readyState).toBe(ws.OPEN)
+	server.loadFromDatabase = async (_roomId: string): Promise<DBLoadResult> => {
+		return { type: 'room_found', snapshot: makeSnapshot(records) }
+	}
 })
 
 const openConnection = async () => {
-	await server.handleConnection({
+	const result = await server.handleConnection({
 		persistenceKey: 'test-persistence-key',
 		sessionKey: 'test-session-key',
 		socket: sockets.server,
 		storeId: 'test-store-id',
 	})
+
+	return result
 }
 
 afterEach(async () => {
@@ -162,4 +187,14 @@ describe('TLServer', () => {
 			},
 		})
 	})
+
+	it('sends a room_not_found when room is not found', async () => {
+		server.loadFromDatabase = async (_roomId: string): Promise<DBLoadResult> => {
+			return { type: 'room_not_found' }
+		}
+
+		const connectionResult = await openConnection()
+
+		expect(connectionResult).toBe('room_not_found')
+	})
 })