Promisify server starting

Summary:
We had a promise for initializing the servers, but it wasn't rejected when server startup failed.
Fixed by resolving / rejecting based on the listen / error events fired from the server.

Reviewed By: passy

Differential Revision: D13762580

fbshipit-source-id: a0d1a56473f84dc416e5ce2de91a53b21574f452

src/__tests__/server.electron.js

@@ -25,13 +25,13 @@ beforeAll(() => {
 
   const logger = initLogger(mockStore);
   server = new Server(logger, mockStore);
-  return server.init();
 });
 
 test('servers starting at ports', done => {
   const serversToBeStarted = new Set([SECURE_PORT, INSECURE_PORT]);
 
-  return new Promise((resolve, reject) => {
+  // Resolve promise when we get a listen event for each port
+  const listenerPromise = new Promise((resolve, reject) => {
     server.addListener('listening', port => {
       if (!serversToBeStarted.has(port)) {
         throw Error(`unknown server started at port ${port}`);
@@ -44,6 +44,11 @@ test('servers starting at ports', done => {
       }
     });
   });
+
+  // Initialise server after the listeners have been setup
+  server.init();
+
+  return listenerPromise;
 });
 
 afterAll(() => {

src/server.js

@@ -40,8 +40,8 @@ type ClientInfo = {|
 
 export default class Server extends EventEmitter {
   connections: Map<string, ClientInfo>;
-  secureServer: RSocketServer;
-  insecureServer: RSocketServer;
+  secureServer: Promise<RSocketServer>;
+  insecureServer: Promise<RSocketServer>;
   certificateProvider: CertificateProvider;
   connectionTracker: ConnectionTracker;
   logger: Logger;
@@ -75,8 +75,13 @@ export default class Server extends EventEmitter {
     return this.initialisePromise;
   }
 
-  startServer(port: number, sslConfig?: SecureServerConfig) {
+  startServer(
+    port: number,
+    sslConfig?: SecureServerConfig,
+  ): Promise<RSocketServer> {
     const server = this;
+    return new Promise((resolve, reject) => {
+      let rsServer;
       const serverFactory = onConnect => {
         const transportServer = sslConfig
           ? tls.createServer(sslConfig, socket => {
@@ -87,6 +92,7 @@ export default class Server extends EventEmitter {
           .on('error', err => {
             server.emit('error', err);
             console.error(`Error opening server on port ${port}`, 'server');
+            reject(err);
           })
           .on('listening', () => {
             console.debug(
@@ -96,10 +102,12 @@ export default class Server extends EventEmitter {
               'server',
             );
             server.emit('listening', port);
+            resolve(rsServer);
           });
         return transportServer;
       };
-    const rsServer = new RSocketServer({
+
+      rsServer = new RSocketServer({
         getRequestHandler: sslConfig
           ? this._trustedRequestHandler
           : this._untrustedRequestHandler,
@@ -108,9 +116,8 @@ export default class Server extends EventEmitter {
           serverFactory: serverFactory,
         }),
       });
-
       rsServer.start();
-    return rsServer;
+    });
   }
 
   _trustedRequestHandler = (conn: RSocket, connectRequest: {data: string}) => {
@@ -244,8 +251,10 @@ export default class Server extends EventEmitter {
   close(): Promise<void> {
     if (this.initialisePromise) {
       return this.initialisePromise.then(_ => {
-        this.secureServer.stop();
-        this.insecureServer.stop();
+        return Promise.all([
+          this.secureServer.then(server => server.stop()),
+          this.insecureServer.then(server => server.stop()),
+        ]).then(() => undefined);
       });
     }
     return Promise.resolve();