fix: Bug fixes, refinements, and lots of tests! (#131)

* Add tests for 100% coverage on synchronize.tsx

* Add tests for 100% coverage on courierSync.ts

* Add tests for 100% coverage in connectionStatus.ts

* Add tests for 100% coverate in settings.ts

* Lint

* Add test for 100% coverage in message.ts

* Expand onboarding to fill the window

* lint

* Hide menubar on About and Libraires windows

Only effects windows and linux.

* Handle close codes from the courier sync websocket, Fixes #129

Anything other 1000 is an error.

* Fix tsc import error

* Provide icons for packaging

Hopefully this fix the app icon on Windows.

Co-authored-by: Gus Narea <[email protected]>

packages/ui/package.json

@@ -71,6 +71,9 @@
   },
   "build": {
     "appId": "tech.relaycorp.gateway",
+    "directories": {
+      "buildResources": "./buildResources"
+    },
     "linux": {
       "category": "Network",
       "icon": "src/electron/assets/logo.png",

packages/ui/src/electron/components/synchronize.spec.tsx

@@ -1,12 +1,48 @@
 import { render } from "@testing-library/react";
 import React from 'react';
+import { CourierSyncError, CourierSyncStatus, synchronizeWithCourier } from '../../ipc/courierSync';
 import Synchronize from './synchronize';
 
+jest.mock('../../ipc/courierSync');
+
 describe('Synchronize', () => {
   test('renders', async () => {
+    (synchronizeWithCourier as jest.Mock).mockReturnValue({
+      promise: (async function* (): AsyncIterable<CourierSyncStatus> {
+        yield CourierSyncStatus.COLLECTING_CARGO;
+      })()
+    })
+
     const onComplete = jest.fn();
     const onReset = jest.fn();
     const el = render(<Synchronize token={"TOKEN"} onComplete={onComplete} onReset={onReset}/>);
     expect(el.container.firstChild).toBeTruthy();
   });
+  test('renders on an error', async () => {
+    (synchronizeWithCourier as jest.Mock).mockReturnValue({
+      promise: (async function* fakeSource(): AsyncIterable<CourierSyncStatus> {
+        throw new CourierSyncError('error');
+      })()
+    })
+
+    const onComplete = jest.fn();
+    const onReset = jest.fn();
+    const el = render(<Synchronize token={"TOKEN"} onComplete={onComplete} onReset={onReset}/>);
+    expect(el.container.firstChild).toBeTruthy();
+  });
+  test('aborts on unmount', async () => {
+    const abort = jest.fn();
+    (synchronizeWithCourier as jest.Mock).mockReturnValue({
+      abort,
+      promise: (async function* fakeSource(): AsyncIterable<CourierSyncStatus> {
+        return;
+      })(),
+    })
+
+    const onComplete = jest.fn();
+    const onReset = jest.fn();
+    const el = render(<Synchronize token={"TOKEN"} onComplete={onComplete} onReset={onReset}/>);
+    el.unmount();
+    expect(abort).toHaveBeenCalledTimes(1);
+  });
 });

packages/ui/src/electron/main.ts

@@ -123,6 +123,7 @@ function showAbout(): void {
     width: 400,
   });
 
+  win.setMenuBarVisibility(false);
   win.loadFile('about.html');
 }
 
@@ -142,6 +143,7 @@ function showLibraries(): void {
     width: 500,
   });
 
+  win.setMenuBarVisibility(false);
   win.loadFile('libraries.html');
 }
 

packages/ui/src/electron/styles.css

@@ -147,13 +147,12 @@ a:hover {
   align-items: center;
 }
 .stepper {
-  width: 770px;
-  height: 460px;
+  height: 100%;
+  width: 100%;
   margin: auto;
   display: flex;
   flex-direction: row;
   background: white;
-  box-shadow: 4px 4px 10px 0px rgb(0, 0, 0, 0.1);
 }
 .stepper .left {
   padding: 10px 30px;
@@ -162,12 +161,14 @@ a:hover {
   align-items: center;
 }
 .stepper .right {
-  flex-grow: 1;
+  box-sizing: border-box;
+  height: 460px;
+  width: 700px;
+  margin: auto;
   padding: 40px 30px;
   display: flex;
   flex-direction: column;
   align-items: flex-start;
-
 }
 .stepper .right h1 {
   text-align: left;

packages/ui/src/ipc/connectionStatus.spec.ts

@@ -2,7 +2,7 @@ import pipe from 'it-pipe';
 import * as itws from 'it-ws';
 import { sleep } from './_utils';
 
-import { asyncIterableToArray, iterableTake } from '../testUtils/iterables';
+import { asyncIterableToArray } from '../testUtils/iterables';
 import { ConnectionStatus, pollConnectionStatus } from './connectionStatus';
 
 jest.mock('it-ws', () => ({
@@ -23,16 +23,13 @@ describe('pollConnectionStatus', () => {
         await sleep(1);
 
         yield 'DISCONNECTED';
+        await sleep(1);
       })(),
     });
 
     jest.setTimeout(10_000);
 
-    const statuses = await pipe(
-      pollConnectionStatus('TOKEN').promise,
-      iterableTake(4),
-      asyncIterableToArray,
-    );
+    const statuses = await pipe(pollConnectionStatus('TOKEN').promise, asyncIterableToArray);
 
     expect(statuses).toEqual([
       ConnectionStatus.UNREGISTERED,
@@ -41,4 +38,20 @@ describe('pollConnectionStatus', () => {
       ConnectionStatus.DISCONNECTED,
     ]);
   });
+  test('should be abortable', async () => {
+    (itws.connect as jest.Mock).mockReturnValue({
+      source: (async function* fakeSource(): AsyncIterable<string> {
+        await sleep(1);
+        yield 'CONNECTED_TO_PUBLIC_GATEWAY';
+      })(),
+    });
+
+    const { promise, abort } = pollConnectionStatus('TOKEN');
+    abort();
+    const handleStatus = jest.fn();
+    for await (const item of promise) {
+      handleStatus(item);
+    }
+    expect(handleStatus).toHaveBeenCalledTimes(0);
+  });
 });

packages/ui/src/ipc/courierSync.spec.ts

@@ -12,6 +12,11 @@ jest.mock('it-ws', () => ({
 describe('synchronizeWithCourier', () => {
   test('should temporarily cycle through all the possible statuses', async () => {
     (itws.connect as jest.Mock).mockReturnValue({
+      socket: {
+        addEventListener: (_name: string, callback: (event: CloseEvent) => void) => {
+          callback(new CloseEvent('close', { code: 1000 }));
+        },
+      },
       source: (async function* fakeSource(): AsyncIterable<string> {
         yield 'COLLECTION';
         await sleep(1);
@@ -34,46 +39,95 @@ describe('synchronizeWithCourier', () => {
       CourierSyncStatus.COMPLETE,
     ]);
   });
-  test('should throw an error on unknown status', async () => {
+  test('should throw an error and not yield an unknown status', async () => {
     (itws.connect as jest.Mock).mockReturnValue({
+      socket: {
+        addEventListener: (_name: string, callback: (event: CloseEvent) => void) => {
+          callback(new CloseEvent('close', { code: 1000 }));
+        },
+      },
       source: (async function* buggySource(): AsyncIterable<string> {
         yield 'UNKNOWN_STATUS';
+        await sleep(1);
       })(),
     });
-    jest.setTimeout(3_000);
 
+    const handleStatus = jest.fn();
     try {
-      await synchronizeWithCourier('TOKEN').promise;
+      for await (const item of synchronizeWithCourier('TOKEN').promise) {
+        handleStatus(item);
+      }
     } catch (err) {
       expect(err).toBeInstanceOf(CourierSyncError);
-      expect(err.message).toEqual('Unknown status: UNKNOWN_STATUS');
+      expect(err.message).toMatch(/Unknown status/i);
+      expect(handleStatus).toHaveBeenCalledTimes(0);
     }
   });
   test('should throw an error on promise rejection status', async () => {
     (itws.connect as jest.Mock).mockReturnValue({
+      socket: {
+        addEventListener: (_name: string, callback: (event: CloseEvent) => void) => {
+          callback(new CloseEvent('close', { code: 1000 }));
+        },
+      },
       source: (async function* failingSource(): AsyncIterable<string> {
         return Promise.reject('REJECTED');
       })(),
     });
-    jest.setTimeout(3_000);
 
+    const handleStatus = jest.fn();
     try {
-      await synchronizeWithCourier('TOKEN').promise;
+      for await (const item of synchronizeWithCourier('TOKEN').promise) {
+        handleStatus(item);
+      }
     } catch (err) {
       expect(err).toBeInstanceOf(CourierSyncError);
+      expect(handleStatus).toHaveBeenCalledTimes(0);
     }
   });
   test('should be abortable', async () => {
     (itws.connect as jest.Mock).mockReturnValue({
+      socket: {
+        addEventListener: (_name: string, callback: (event: CloseEvent) => void) => {
+          callback(new CloseEvent('close', { code: 1000 }));
+        },
+      },
       source: (async function* fakeSource(): AsyncIterable<string> {
-        await sleep(5);
-        yield 'COLLECTING_CARGO';
+        await sleep(1);
+        yield 'COLLECTION';
       })(),
     });
-    jest.setTimeout(1_000);
 
     const { promise, abort } = synchronizeWithCourier('TOKEN');
     abort();
-    await promise;
+    const handleStatus = jest.fn();
+    for await (const item of promise) {
+      handleStatus(item);
+    }
+    expect(handleStatus).toHaveBeenCalledTimes(0);
+  });
+  test('should throw an exception on a closing error code', async () => {
+    (itws.connect as jest.Mock).mockReturnValue({
+      socket: {
+        addEventListener: (_name: string, callback: (event: CloseEvent) => void) => {
+          callback(new CloseEvent('close', { code: 1011 }));
+        },
+      },
+      source: (async function* fakeSource(): AsyncIterable<string> {
+        await sleep(1);
+        yield 'COLLECTION';
+      })(),
+    });
+
+    const { promise } = synchronizeWithCourier('TOKEN');
+    const handleStatus = jest.fn();
+    try {
+      for await (const item of promise) {
+        handleStatus(item);
+      }
+    } catch (err) {
+      expect(err).toBeInstanceOf(CourierSyncError);
+      expect(err.message).toMatch(/1011/i);
+    }
   });
 });

packages/ui/src/ipc/courierSync.ts

@@ -1,5 +1,6 @@
 import abortable from 'abortable-iterator';
 import { connect } from 'it-ws';
+import { CloseEvent } from 'ws';
 import PrivateGatewayError from '../PrivateGatewayError';
 
 export enum CourierSyncStatus {
@@ -44,6 +45,11 @@ async function* _synchronizeWithCourier(token: string): AsyncIterable<CourierSyn
   try {
     const WS_URL = 'ws://127.0.0.1:13276/_control/courier-sync?auth=' + token;
     const stream = connect(WS_URL, { binary: true });
+    let closeEventCode = 0;
+    stream.socket.addEventListener('close', (event: CloseEvent) => {
+      closeEventCode = event.code;
+    });
+
     for await (const buffer of stream.source) {
       const name = buffer.toString();
       switch (name) {
@@ -60,6 +66,9 @@ async function* _synchronizeWithCourier(token: string): AsyncIterable<CourierSyn
           throw new CourierSyncError(`Unknown status: ${name}`);
       }
     }
+    if (closeEventCode !== 1000) {
+      throw new CourierSyncError(`Socket error: ${closeEventCode}`);
+    }
     // Server does not send this one, but the UI is waiting for it
     yield CourierSyncStatus.COMPLETE;
   } catch (err) {

packages/ui/src/ipc/message.spec.ts

@@ -0,0 +1,14 @@
+import { ServerMessage, ServerMessageType } from '../ipc/message';
+
+describe('ServerMessage', () => {
+  test('is defined', async () => {
+    function handler(message: ServerMessage): ServerMessage {
+      return message;
+    }
+    const serverMessage = handler({
+      type: ServerMessageType.TOKEN_MESSAGE,
+      value: 'authtoken',
+    });
+    expect(serverMessage.value).toEqual('authtoken');
+  });
+});

packages/ui/src/ipc/settings.spec.ts

@@ -22,10 +22,22 @@ describe('getPublicGatewayAddress', () => {
 
     expect(fetchMock.lastUrl()).toEqual('http://127.0.0.1:13276/_control/public-gateway');
   });
+  test('should throw SettingError on a server error message', async () => {
+    fetchMock.get(request, {
+      body: { message: 'error message' },
+      status: 500,
+    });
+    try {
+      await getPublicGatewayAddress('TOKEN');
+    } catch (error) {
+      expect(error).toBeInstanceOf(SettingError);
+      expect(error.message).toEqual('error message');
+    }
+  });
   test('should throw SettingError on a random response', async () => {
     fetchMock.get(request, {
+      body: {},
       status: 404,
-      statusText: 'Unknown',
     });
     try {
       await getPublicGatewayAddress('TOKEN');
@@ -72,10 +84,21 @@ describe('migratePublicGatewayAddress', () => {
       expect(fetchMock.lastUrl()).toEqual('http://127.0.0.1:13276/_control/public-gateway');
     }
   });
+  test('should throw SettingError on a random response', async () => {
+    fetchMock.put(request, {
+      body: {},
+      status: 400,
+    });
+    try {
+      await migratePublicGatewayAddress('kings-landing.relaycorp.cloud', 'TOKEN');
+    } catch (error) {
+      expect(error).toBeInstanceOf(SettingError);
+      expect(fetchMock.lastUrl()).toEqual('http://127.0.0.1:13276/_control/public-gateway');
+    }
+  });
   test('should throw SettingError on a random response', async () => {
     fetchMock.put(request, {
       status: 404,
-      statusText: 'Unknown',
     });
     try {
       await migratePublicGatewayAddress('kings-landing.relaycorp.cloud', 'TOKEN');

packages/ui/src/types/it-ws.d.ts

@@ -1,10 +1,12 @@
 declare module 'it-ws' {
   import { Duplex } from 'stream';
+  import WebSocket from 'ws';
   interface Options {
     readonly binary: boolean;
   }
   interface SocketStream {
     readonly source: AsyncIterable<any>;
+    readonly socket: WebSocket;
   }
 
   export function connect(url: string, options?: Options): SocketStream;