Add some failure codes when adding a server

Partly addresses #925.

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,7 +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-with-code.js'
+import { ErrorWithCode } from './lib/error.js'
 import { MEMBER_ROLE_ID, ROLES, isRoleIdForNewInvite } from './roles.js'
 import { wsCoreReplicator } from './server/ws-core-replicator.js'
 /**
@@ -265,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]
@@ -375,7 +386,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

@@ -180,10 +180,17 @@ test("fails if first request succeeds but sync doesn't", async (t) => {
       project.$member.addServerPeer(serverBaseUrl, {
         dangerouslyAllowInsecureConnections: true,
       }),
-    {
-      // TODO
-      // code: 'INVALID_SERVER_RESPONSE',
-      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
     }
   )
 })

test/lib/error.js

@@ -0,0 +1,66 @@
+import assert from 'node:assert/strict'
+import test, { describe } from 'node:test'
+import { ErrorWithCode, getErrorMessage } from '../../src/lib/error.js'
+
+describe('ErrorWithCode', () => {
+  test('ErrorWithCode with two arguments', () => {
+    const err = new ErrorWithCode('MY_CODE', 'my message')
+    assert.equal(err.code, 'MY_CODE')
+    assert.equal(err.message, 'my message')
+    assert(err instanceof Error)
+  })
+
+  test('ErrorWithCode with three arguments', () => {
+    const otherError = new Error('hello')
+    const err = new ErrorWithCode('MY_CODE', 'my message', {
+      cause: otherError,
+    })
+    assert.equal(err.code, 'MY_CODE')
+    assert.equal(err.message, 'my message')
+    assert.equal(err.cause, otherError)
+    assert(err instanceof Error)
+  })
+})
+
+describe('getErrorMessage', () => {
+  test('from objects without a string message', () => {
+    const testCases = [
+      undefined,
+      null,
+      ['ignored'],
+      { message: 123 },
+      {
+        get message() {
+          throw new Error('this should not crash')
+        },
+      },
+    ]
+
+    for (const testCase of testCases) {
+      assert.equal(getErrorMessage(testCase), 'unknown error')
+    }
+  })
+
+  test('from objects with a string message', () => {
+    class WithInheritedMessage {
+      get message() {
+        return 'foo'
+      }
+    }
+
+    const testCases = [
+      { message: 'foo' },
+      new Error('foo'),
+      {
+        get message() {
+          return 'foo'
+        },
+      },
+      new WithInheritedMessage(),
+    ]
+
+    for (const testCase of testCases) {
+      assert.equal(getErrorMessage(testCase), 'foo')
+    }
+  })
+})