Merge branch 'server' into feat/server-plugin

src/lib/error.js

@@ -0,0 +1,47 @@
+/**
+ * Create an `Error` with a `code` property.
+ *
+ * @example
+ * const err = new ErrorWithCode('INVALID_DATA', 'data was invalid')
+ * err.message
+ * // => 'data was invalid'
+ * err.code
+ * // => 'INVALID_DATA'
+ */
+export class ErrorWithCode extends Error {
+  /**
+   * @param {string} code
+   * @param {string} message
+   * @param {object} [options]
+   * @param {unknown} [options.cause]
+   */
+  constructor(code, message, options) {
+    super(message, options)
+    /** @readonly */ this.code = code
+  }
+}
+
+/**
+ * Get the error message from an object if possible.
+ * Otherwise, stringify the argument.
+ *
+ * @param {unknown} maybeError
+ * @returns {string}
+ * @example
+ * try {
+ *   // do something
+ * } catch (err) {
+ *   console.error(getErrorMessage(err))
+ * }
+ */
+export function getErrorMessage(maybeError) {
+  if (maybeError && typeof maybeError === 'object' && 'message' in maybeError) {
+    try {
+      const { message } = maybeError
+      if (typeof message === 'string') return message
+    } catch (_err) {
+      // Ignored
+    }
+  }
+  return 'unknown error'
+}

src/member-api.js

@@ -16,6 +16,7 @@ import { keyBy } from './lib/key-by.js'
 import { abortSignalAny } from './lib/ponyfills.js'
 import timingSafeEqual from './lib/timing-safe-equal.js'
 import { isHostnameIpAddress } from './lib/is-hostname-ip-address.js'
+import { ErrorWithCode } from './lib/error.js'
 import { MEMBER_ROLE_ID, ROLES, isRoleIdForNewInvite } from './roles.js'
 import { wsCoreReplicator } from './server/ws-core-replicator.js'
 /**
@@ -264,6 +265,17 @@ export class MemberApi extends TypedEmitter {
   /**
    * Add a server peer.
    *
+   * Can reject with any of the following error codes (accessed via `err.code`):
+   *
+   * - `INVALID_URL`: the base URL is invalid, likely due to user error.
+   * - `NETWORK_ERROR`: there was an issue connecting to the server. Is the
+   *   device online? Is the server online?
+   * - `INVALID_SERVER_RESPONSE`: we connected to the server but it returned
+   *   an unexpected response. Is the server running a compatible version of
+   *   CoMapeo Cloud?
+   *
+   * If `err.code` is not specified, that indicates a bug in this module.
+   *
    * @param {string} baseUrl
    * @param {object} [options]
    * @param {boolean} [options.dangerouslyAllowInsecureConnections]
@@ -273,10 +285,11 @@ export class MemberApi extends TypedEmitter {
     baseUrl,
     { dangerouslyAllowInsecureConnections = false } = {}
   ) {
-    assert(
-      isValidServerBaseUrl(baseUrl, { dangerouslyAllowInsecureConnections }),
-      'Base URL is invalid'
-    )
+    if (
+      !isValidServerBaseUrl(baseUrl, { dangerouslyAllowInsecureConnections })
+    ) {
+      throw new ErrorWithCode('INVALID_URL', 'Server base URL is invalid')
+    }
 
     const { serverDeviceId } = await this.#addServerToProject(baseUrl)
 
@@ -319,13 +332,15 @@ export class MemberApi extends TypedEmitter {
         err && typeof err === 'object' && 'message' in err
           ? err.message
           : String(err)
-      throw new Error(
+      throw new ErrorWithCode(
+        'NETWORK_ERROR',
         `Failed to add server peer due to network error: ${message}`
       )
     }
 
     if (response.status !== 200 && response.status !== 201) {
-      throw new Error(
+      throw new ErrorWithCode(
+        'INVALID_SERVER_RESPONSE',
         `Failed to add server peer due to HTTP status code ${response.status}`
       )
     }
@@ -344,12 +359,22 @@ export class MemberApi extends TypedEmitter {
       )
       return { serverDeviceId: responseBody.data.deviceId }
     } catch (err) {
-      throw new Error(
+      throw new ErrorWithCode(
+        'INVALID_SERVER_RESPONSE',
         "Failed to add server peer because we couldn't parse the response"
       )
     }
   }
 
+  /**
+   * @param {string} serverDeviceId
+   * @returns {Promise<void>}
+   */
+  async removeServerPeer(serverDeviceId) {
+    // TODO
+    console.log({ serverDeviceId })
+  }
+
   /**
    * @param {object} options
    * @param {string} options.baseUrl
@@ -370,7 +395,23 @@ export class MemberApi extends TypedEmitter {
         : 'wss:'
 
     const websocket = new WebSocket(websocketUrl)
-    await pEvent(websocket, 'open')
+
+    try {
+      await pEvent(websocket, 'open', { rejectionEvents: ['error'] })
+    } catch (rejectionEvent) {
+      throw new ErrorWithCode(
+        // It's difficult for us to reliably disambiguate between "network error"
+        // and "invalid response from server" here, so we just say it was an
+        // invalid server response.
+        'INVALID_SERVER_RESPONSE',
+        'Failed to open the socket',
+        rejectionEvent &&
+        typeof rejectionEvent === 'object' &&
+        'error' in rejectionEvent
+          ? { cause: rejectionEvent.error }
+          : { cause: rejectionEvent }
+      )
+    }
 
     const onErrorPromise = pEvent(websocket, 'error')
 

test-e2e/server.js

@@ -5,7 +5,7 @@ import createFastify from 'fastify'
 import assert from 'node:assert/strict'
 import test from 'node:test'
 import { pEvent } from 'p-event'
-import { MEMBER_ROLE_ID } from '../src/roles.js'
+import { LEFT_ROLE_ID, MEMBER_ROLE_ID } from '../src/roles.js'
 import comapeoServer from '../src/server/app.js'
 import {
   connectPeers,
@@ -18,6 +18,7 @@ import {
 } from './utils.js'
 /** @import { MapeoManager } from '../src/mapeo-manager.js' */
 /** @import { MapeoProject } from '../src/mapeo-project.js' */
+/** @import { MemberInfo } from '../src/member-api.js' */
 /** @import { State as SyncState } from '../src/sync/sync-api.js' */
 
 test('invalid base URLs', async (t) => {
@@ -55,16 +56,16 @@ test('invalid base URLs', async (t) => {
     invalidUrls.map((url) =>
       assert.rejects(
         () => project.$member.addServerPeer(url),
-        /base url is invalid/i,
+        {
+          code: 'INVALID_URL',
+          message: /base url is invalid/i,
+        },
         `${url} should be invalid`
       )
     )
   )
 
-  const hasServerPeer = (await project.$member.getMany()).some(
-    (member) => member.deviceType === 'selfHostedServer'
-  )
-  assert(!hasServerPeer, 'no server peers should be added')
+  assert(!(await findServerPeer(project)), 'no server peers should be added')
 })
 
 test("fails if we can't connect to the server", async (t) => {
@@ -79,6 +80,7 @@ test("fails if we can't connect to the server", async (t) => {
         dangerouslyAllowInsecureConnections: true,
       }),
     {
+      code: 'NETWORK_ERROR',
       message: /Failed to add server peer due to network error/,
     }
   )
@@ -108,6 +110,7 @@ test(
                 dangerouslyAllowInsecureConnections: true,
               }),
             {
+              code: 'INVALID_SERVER_RESPONSE',
               message: `Failed to add server peer due to HTTP status code ${statusCode}`,
             }
           )
@@ -147,6 +150,7 @@ test(
                 dangerouslyAllowInsecureConnections: true,
               }),
             {
+              code: 'INVALID_SERVER_RESPONSE',
               message:
                 "Failed to add server peer because we couldn't parse the response",
             }
@@ -174,8 +178,17 @@ test("fails if first request succeeds but sync doesn't", async (t) => {
       project.$member.addServerPeer(serverBaseUrl, {
         dangerouslyAllowInsecureConnections: true,
       }),
-    {
-      message: /404/,
+    (err) => {
+      assert(err instanceof Error, 'receives an error')
+      assert('code' in err, 'gets an error code')
+      assert.equal(
+        err.code,
+        'INVALID_SERVER_RESPONSE',
+        'gets the correct error code'
+      )
+      assert(err.cause instanceof Error, 'error has a cause')
+      assert(err.cause.message.includes('404'), 'error cause is an HTTP 404')
+      return true
     }
   )
 })
@@ -187,18 +200,13 @@ test('adding a server peer', async (t) => {
 
   const serverBaseUrl = await createTestServer(t)
 
-  const hasServerPeerBeforeAdding = (await project.$member.getMany()).some(
-    (member) => member.deviceType === 'selfHostedServer'
-  )
-  assert(!hasServerPeerBeforeAdding, 'no server peers before adding')
+  assert(!(await findServerPeer(project)), 'no server peers before adding')
 
   await project.$member.addServerPeer(serverBaseUrl, {
     dangerouslyAllowInsecureConnections: true,
   })
 
-  const serverPeer = (await project.$member.getMany()).find(
-    (member) => member.deviceType === 'selfHostedServer'
-  )
+  const serverPeer = await findServerPeer(project)
   assert(serverPeer, 'expected a server peer to be found by the client')
   assert.equal(serverPeer.name, 'test server', 'server peers have name')
   assert.equal(
@@ -213,6 +221,30 @@ test('adding a server peer', async (t) => {
   )
 })
 
+test.skip('removing a server peer', async (t) => {
+  const manager = createManager('device0', t)
+  const projectId = await manager.createProject()
+  const project = await manager.getProject(projectId)
+
+  const serverBaseUrl = await createTestServer(t)
+
+  await project.$member.addServerPeer(serverBaseUrl, {
+    dangerouslyAllowInsecureConnections: true,
+  })
+
+  const serverPeer = await findServerPeer(project)
+  assert(serverPeer, 'server peer should be added')
+  await project.$member.removeServerPeer(serverPeer.deviceId)
+
+  assert.equal(
+    (await findServerPeer(project))?.role.roleId,
+    LEFT_ROLE_ID,
+    'we should believe the server is gone'
+  )
+
+  // TODO: ensure no connections are made
+})
+
 test("can't add a server to two different projects", async (t) => {
   const [managerA, managerB] = await createManagers(2, t, 'mobile')
   const projectIdA = await managerA.createProject()
@@ -247,15 +279,12 @@ test('data can be synced via a server', async (t) => {
   await managerAProject.$member.addServerPeer(serverBaseUrl, {
     dangerouslyAllowInsecureConnections: true,
   })
-  const serverDeviceIdPromise = managerAProject.$member
-    .getMany()
-    .then((members) => {
-      const serverMember = members.find(
-        (member) => member.deviceType === 'selfHostedServer'
-      )
+  const serverDeviceIdPromise = findServerPeer(managerAProject).then(
+    (serverMember) => {
       assert(serverMember, 'Manager A must have a server member')
       return serverMember.deviceId
-    })
+    }
+  )
 
   // Add Manager B to project
   const disconnectManagers = connectPeers(managers)
@@ -268,10 +297,7 @@ test('data can be synced via a server', async (t) => {
   // Sync managers to tell Manager B about the server
   const projects = [managerAProject, managerBProject]
   await waitForSync(projects, 'initial')
-  const managerBMembers = await managerBProject.$member.getMany()
-  const serverPeer = managerBMembers.find(
-    (member) => member.deviceType === 'selfHostedServer'
-  )
+  const serverPeer = await findServerPeer(managerBProject)
   assert(serverPeer, 'expected a server peer to be found by the client')
 
   // Manager A adds data that Manager B doesn't know about
@@ -361,6 +387,16 @@ async function createLocalTestServer(t) {
   return address
 }
 
+/**
+ * @param {MapeoProject} project
+ * @returns {Promise<undefined | MemberInfo>}
+ */
+async function findServerPeer(project) {
+  return (await project.$member.getMany()).find(
+    (member) => member.deviceType === 'selfHostedServer'
+  )
+}
+
 /**
  * @param {MapeoManager} manager
  * @returns {Promise<void>}