Support error responses to requests for a message channel

Allow the host frame to explicitly reject a request for a message channel by
setting the `Message.error` property. Previously other frames could only wait
for requests to time out. This allows the sidebar to display an error much
sooner if it gets reloaded and cannot reconnect to the host frame.

Part of https://github.com/hypothesis/client/issues/4095

src/shared/messaging/port-finder.ts

 import type { Destroyable } from '../../types/annotator';
 import { ListenerCollection } from '../listener-collection';
 import { generateHexString } from '../random';
-import { isMessageEqual } from './port-util';
+import { isMessage } from './port-util';
 import type { Frame } from './port-util';
 
 /** Timeout for waiting for the host frame to respond to a port request. */
@@ -21,6 +21,13 @@ export type Options = {
   hostFrame: Window;
 };
 
+/** Error thrown when a {@link PortFinder.discover} request fails. */
+export class PortRequestError extends Error {
+  constructor(message: string) {
+    super(message);
+  }
+}
+
 /**
  * PortFinder is used by non-host frames in the client to establish a
  * MessagePort-based connection to other frames. It is used together with
@@ -78,7 +85,7 @@ export class PortFinder implements Destroyable {
       const timeoutId = setTimeout(() => {
         clearInterval(intervalId);
         reject(
-          new Error(
+          new PortRequestError(
             `Unable to establish ${this._source}-${target} communication channel`,
           ),
         );
@@ -86,18 +93,36 @@ export class PortFinder implements Destroyable {
 
       const listenerId = this._listeners.add(window, 'message', event => {
         const { data, ports } = event;
+
+        // Ignore messages that are:
+        //
+        // - Not related to port discovery
+        // - Not a response to the request we sent above. Note that the host
+        //   frame may be the same as the current window, since eg. the host
+        //   frame can also be a guest frame. Therefore we check `data.type` as
+        //   well to make sure this is a response.
         if (
-          isMessageEqual(data, {
-            frame1: this._source,
-            frame2: target,
-            type: 'offer',
-            requestId,
-          })
+          !isMessage(data) ||
+          data.requestId !== requestId ||
+          data.type === 'request'
         ) {
-          clearInterval(intervalId);
-          clearTimeout(timeoutId);
-          this._listeners.remove(listenerId);
+          return;
+        }
+
+        clearInterval(intervalId);
+        clearTimeout(timeoutId);
+        this._listeners.remove(listenerId);
+
+        if (typeof data.error === 'string') {
+          reject(new PortRequestError(data.error));
+        } else if (ports.length > 0) {
           resolve(ports[0]);
+        } else {
+          reject(
+            new PortRequestError(
+              `${this._source}-${target} port request failed`,
+            ),
+          );
         }
       });
 

src/shared/messaging/port-provider.ts

@@ -74,6 +74,8 @@ export class PortProvider implements Destroyable {
   private _handledRequests: Set<string>;
 
   private _listeners: ListenerCollection;
+
+  // Message channels for connections from Hypothesis apps to the host frame.
   private _sidebarHostChannel: MessageChannel;
   private _sidebarConnected: boolean;
 
@@ -198,6 +200,18 @@ export class PortProvider implements Destroyable {
           ? this._sidebarHostChannel
           : new MessageChannel();
 
+      // The message that is sent to the target frame that the source wants to
+      // connect to, as well as the source frame requesting the connection.
+      // Each message is accompanied by a port for the appropriate end of the
+      // connection.
+      const message: Message = {
+        frame1,
+        frame2,
+        type: 'offer',
+        requestId,
+        sourceId,
+      };
+
       // The sidebar can only connect once. It might try to connect a second
       // time if something causes the iframe to reload. We can't recover from
       // this yet. Instead we just log a warning here. The port discovery
@@ -208,17 +222,13 @@ export class PortProvider implements Destroyable {
           console.warn(
             'Ignoring second request from Hypothesis sidebar to connect to host frame',
           );
+          message.error = 'Received duplicate port request';
+          source.postMessage(message, targetOrigin);
           return;
         }
         this._sidebarConnected = true;
       }
 
-      // The message that is sent to the target frame that the source wants to
-      // connect to, as well as the source frame requesting the connection.
-      // Each message is accompanied by a port for the appropriate end of the
-      // connection.
-      const message = { frame1, frame2, type: 'offer', requestId, sourceId };
-
       source.postMessage(message, targetOrigin, [messageChannel.port1]);
 
       if (frame2 === 'sidebar') {

src/shared/messaging/port-util.ts

@@ -19,6 +19,9 @@ export type Message = {
    */
   type: 'offer' | 'request';
 
+  /** If set on an `offer` message, indicates that the request was rejected. */
+  error?: string;
+
   /**
    * ID of the request. Used to associate "offer" messages with their
    * corresponding "request" messages.

src/shared/messaging/test/port-finder-test.js

@@ -3,6 +3,7 @@ import {
   MAX_WAIT_FOR_PORT,
   POLLING_INTERVAL_FOR_PORT,
   PortFinder,
+  PortRequestError,
   $imports,
 } from '../port-finder';
 
@@ -79,6 +80,7 @@ describe('PortFinder', () => {
 
       await delay(0);
 
+      assert.instanceOf(error, PortRequestError);
       assert.equal(
         error.message,
         'Unable to establish guest-host communication channel',
@@ -167,10 +169,53 @@ describe('PortFinder', () => {
 
       await delay(0);
 
+      assert.instanceOf(error, PortRequestError);
       assert.equal(
         error.message,
         'Unable to establish guest-host communication channel',
       );
     });
+
+    it('rejects if host frame rejects port request with an explicit reason', async () => {
+      let error;
+
+      const done = portFinder.discover('host').catch(e => (error = e));
+
+      sendPortProviderOffer({
+        data: {
+          frame1: 'guest',
+          frame2: 'host',
+          requestId,
+          type: 'offer',
+          error: 'Host frame says no',
+        },
+      });
+
+      await done;
+
+      assert.instanceOf(error, PortRequestError);
+      assert.equal(error.message, 'Host frame says no');
+    });
+
+    it('rejects if host frame does not send a port', async () => {
+      let error;
+
+      const done = portFinder.discover('host').catch(e => (error = e));
+
+      sendPortProviderOffer({
+        // This is a valid response, except that the ports are missing.
+        data: {
+          frame1: 'guest',
+          frame2: 'host',
+          requestId,
+          type: 'offer',
+        },
+      });
+
+      await done;
+
+      assert.instanceOf(error, PortRequestError);
+      assert.equal(error.message, 'guest-host port request failed');
+    });
   });
 });

src/shared/messaging/test/port-provider-test.js

@@ -190,7 +190,15 @@ describe('PortProvider', () => {
         await sendPortFinderRequest({
           data: { ...data, requestId: 'second' },
         });
-        assert.notCalled(window.postMessage);
+        assert.calledWith(
+          window.postMessage,
+          sinon.match({
+            ...data,
+            type: 'offer',
+            error: 'Received duplicate port request',
+          }),
+          window.location.origin,
+        );
         assert.calledWith(
           warnStub,
           'Ignoring second request from Hypothesis sidebar to connect to host frame',