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,42 +75,49 @@ export default class Server extends EventEmitter {
  return this.initialisePromise;
  }
 
- startServer(port: number, sslConfig?: SecureServerConfig) {
+ startServer(
+ port: number,
+ sslConfig?: SecureServerConfig,
+ ): Promise<RSocketServer> {
  const server = this;
- const serverFactory = onConnect => {
- const transportServer = sslConfig
- ? tls.createServer(sslConfig, socket => {
- onConnect(socket);
+ return new Promise((resolve, reject) => {
+ let rsServer;
+ const serverFactory = onConnect => {
+ const transportServer = sslConfig
+ ? tls.createServer(sslConfig, socket => {
+ onConnect(socket);
+ })
+ : net.createServer(onConnect);
+ transportServer
+ .on('error', err => {
+ server.emit('error', err);
+ console.error(`Error opening server on port ${port}`, 'server');
+ reject(err);
  })
- : net.createServer(onConnect);
- transportServer
- .on('error', err => {
- server.emit('error', err);
- console.error(`Error opening server on port ${port}`, 'server');
- })
- .on('listening', () => {
- console.debug(
- `${
- sslConfig ? 'Secure' : 'Certificate'
- } server started on port ${port}`,
- 'server',
- );
- server.emit('listening', port);
- });
- return transportServer;
- };
- const rsServer = new RSocketServer({
- getRequestHandler: sslConfig
- ? this._trustedRequestHandler
- : this._untrustedRequestHandler,
- transport: new RSocketTCPServer({
- port: port,
- serverFactory: serverFactory,
- }),
+ .on('listening', () => {
+ console.debug(
+ `${
+ sslConfig ? 'Secure' : 'Certificate'
+ } server started on port ${port}`,
+ 'server',
+ );
+ server.emit('listening', port);
+ resolve(rsServer);
+ });
+ return transportServer;
+ };
+
+ rsServer = new RSocketServer({
+ getRequestHandler: sslConfig
+ ? this._trustedRequestHandler
+ : this._untrustedRequestHandler,
+ transport: new RSocketTCPServer({
+ port: port,
+ serverFactory: serverFactory,
+ }),
+ });
+ rsServer.start();
  });
-
- 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();