[WIP] refactor(hapi): Upgrade to hapi 17

.eslintrc

@@ -5,3 +5,6 @@ extends: plugin:fxa/server
 rules:
   handle-callback-err: 0
   semi: [2, "always"]
+
+parserOptions:
+  ecmaVersion: 2018

bin/_static.js

@@ -4,13 +4,15 @@
 
 const config = require('../lib/config').getProperties();
 const logger = require('../lib/logging')('bin._static');
-const server = require('../lib/server/_static').create();
+
+async function create() {
+  const server = await require('../lib/server/_static').create();
+  await server.start();
+  logger.info('listening', server.info.uri);
+}
+create();
 
 if (config.env !== 'development') {
   logger.warn('sanity-check', 'static bin should only be used for local dev!');
 }
 
-
-server.start(function() {
-  logger.info('listening', server.info.uri);
-});

bin/server.js

@@ -9,21 +9,30 @@ require('../lib/newrelic')();
 const configuration = require('../lib/config');
 const db = require('../lib/db');
 const logger = require('../lib/logging')('bin.server');
-const server = require('../lib/server').create();
-const events = require('../lib/events')(server);
 
 // The stringify/parse is to force the output back to unindented json.
 logger.info('config', JSON.stringify(JSON.parse(configuration.toString())));
-db.ping().done(function() {
-  server.start(function(err) {
-    if (err) {
-      logger.critical('server.start', err);
-      process.exit(1);
-    }
+
+
+async function start() {
+  const server = await require('../lib/server').create();
+
+  try {
+    await db.ping();
+  } catch (err) {
+    logger.critical('db.ping', err);
+    process.exit(2);
+  }
+  try {
+    await server.start();
     logger.info('listening', server.info.uri);
-  });
-  events.start();
-}, function(err) {
-  logger.critical('db.ping', err);
-  process.exit(2);
-});
+    const events = require('../lib/events')(server);
+    events.start();
+  } catch (err) {
+    logger.critical('server.start', err);
+    process.exit(1);
+  }
+}
+
+start();
+

bin/worker.js

@@ -7,8 +7,13 @@
 require('../lib/newrelic')();
 
 const logger = require('../lib/logging')('bin.worker');
-const server = require('../lib/server/worker').create();
 
-server.start(function() {
+async function start() {
+  const server = await require('../lib/server/worker').create();
+
+  await server.start();
   logger.info('listening', server.info.uri);
-});
+}
+
+start();
+

lib/batch.js

@@ -7,12 +7,6 @@ const Boom = require('boom');
 const logger = require('./logging')('batch');
 const P = require('./promise');
 
-function inject(server, options) {
-  return new P(function(resolve) {
-    server.inject(options, resolve);
-  });
-}
-
 // Make multiple internal requests to routes, and merge their responses
 // into a single object.
 //
@@ -46,7 +40,7 @@ function batch(request, routeFieldsMap) {
   let numForbidden = 0;
   const routeFieldsKeys = Object.keys(routeFieldsMap);
   return P.each(routeFieldsKeys, url => {
-    return inject(request.server, {
+    return request.server.inject({
       allowInternals: true,
       method: 'get',
       url: url,

lib/events.js

@@ -45,9 +45,10 @@ module.exports = function (server) {
   function primaryEmailChanged(message) {
     var userId = getUserId(message);
     return P.resolve().then(() => {
-      server.methods.profileCache.drop(userId, () => {
-        logger.info('primaryEmailChanged:cacheCleared', {uid: userId});
-      });
+      server.methods.profileCache.drop(userId)
+        .then(() => {
+          logger.info('primaryEmailChanged:cacheCleared', {uid: userId});
+        });
     }).then(function () {
       logger.info(message.event, {uid: userId});
     });
@@ -56,9 +57,10 @@ module.exports = function (server) {
   function profileDataChanged(message) {
     var userId = getUserId(message);
     return P.resolve().then(function () {
-      server.methods.profileCache.drop(userId, () => {
-        logger.info('profileDataChanged:cacheCleared', {uid: userId});
-      });
+      server.methods.profileCache.drop(userId)
+        .then(() => {
+          logger.info('profileDataChanged:cacheCleared', {uid: userId});
+        });
     }).then(function () {
       logger.info(message.event, {uid: userId});
     });

lib/profileCache.js

@@ -51,28 +51,27 @@ function getProfileCacheKey(req) {
 }
 
 
-module.exports = function profileCache(server, options, next) {
+module.exports = function profileCache(server, options) {
 
   // Fetch all available profile data given the scopes on the request,
   // using cached data if possible.
 
-  server.method('profileCache.get', (req, next) => {
-    batch(req, {
+  server.method('profileCache.get', (req) => {
+    return batch(req, {
       '/v1/_core_profile': true,
       '/v1/uid': true,
       '/v1/avatar': ['avatar', 'avatarDefault'],
       '/v1/display_name': true
     })
-    .then(result => {
-      // Only cache the result if we can produce a suitable cache key.
-      const ttl = getProfileCacheKey(req) ? undefined : 0;
-      return next(null, result, ttl);
-    })
-    .catch(next);
+      .then((result) => {
+        // Only cache the result if we can produce a suitable cache key.
+        const ttl = getProfileCacheKey(req) ? undefined : 0;
+        return { result, ttl };
+      });
   }, {
     cache: {
       expiresIn: options.expiresIn,
-      generateTimeout: options.generateTimeout
+      generateTimeout: options.generateTimeout || 100
     },
     generateKey: (req) => {
       return getProfileCacheKey(req) || 'can-not-cache';
@@ -81,24 +80,20 @@ module.exports = function profileCache(server, options, next) {
 
   // Drop any cached profile data for the given user.
 
-  server.method('profileCache.drop', (uid, next) => {
+  server.method('profileCache.drop', (uid) => {
     // To work transparently with hapi's caching and `getProfileCacheKey` above,
     // we make a bunch of synthetic request objects on which to drop the
     // cache, one for each possible set of scopes in the cache.
-    return P.each(CACHEABLE_SCOPES, scope => {
+    return P.each(CACHEABLE_SCOPES, (scope) => {
       const req = { auth: { credentials: {
         user: uid,
         scope: scope
       }}};
-      return P.fromCallback(cb => {
-        server.methods.profileCache.get.cache.drop(req, cb);
+      return server.methods.profileCache.get.cache.drop(req);
+    })
+      .catch((err) => {
+        throw err;
       });
-    }).asCallback(next);
   });
-
-  next();
 };
 
-module.exports.attributes = {
-  name: 'fxa-profile-cache'
-};

lib/routes/_core_profile.js

@@ -30,62 +30,66 @@ module.exports = {
       twoFactorAuthentication: Joi.boolean().optional()
     }
   },
-  handler: function _core_profile(req, reply) {
-    request.get(AUTH_SERVER_URL, {
-      headers: {
-        Authorization: 'Bearer ' + req.auth.credentials.token
-      },
-      json: true
-    }, (err, res, body) => {
-      if (err) {
-        logger.error('request.auth_server.network', err);
-        return reply(new AppError.authError('network error'));
-      }
-      if (res.statusCode >= 400) {
-        body = body && body.code ? body : { code: res.statusCode };
-        if (res.statusCode >= 500) {
-          logger.error('request.auth_server.fail', body);
-          return reply(new AppError.authError('auth-server server error'));
-        }
-        // Return Unauthorized if the token turned out to be invalid,
-        // or if the account has been deleted on the auth-server.
-        // (we can still have valid oauth tokens for deleted accounts,
-        // because distributed state).
-        if (body.code === 401 || body.errno === 102) {
-          logger.info('request.auth_server.fail', body);
-          return reply(new AppError.unauthorized(body.message));
-        }
-        // There should be no other 400-level errors, unless we're
-        // sending a badly-formed request of our own.  That warrants
-        // an "Internal Server Error" on our part.
-        logger.error('request.auth_server.fail', body);
-        return reply(new AppError({
-          code: 500,
-          message: 'error communicating with auth server'
-        }));
-      }
+  handler: async function _core_profile(req) {
+    function makeReq() {
+      return new Promise((resolve, reject) => {
+        request.get(AUTH_SERVER_URL, {
+          headers: {
+            Authorization: 'Bearer ' + req.auth.credentials.token
+          },
+          json: true
+        }, (err, res, body) => {
+          if (err) {
+            logger.error('request.auth_server.network', err);
+            return reject(new AppError.authError('network error'));
+          }
+          if (res.statusCode >= 400) {
+            body = body && body.code ? body : { code: res.statusCode };
+            if (res.statusCode >= 500) {
+              logger.error('request.auth_server.fail', body);
+              return reject(AppError.authError('auth-server server error'));
+            }
+            // Return Unauthorized if the token turned out to be invalid,
+            // or if the account has been deleted on the auth-server.
+            // (we can still have valid oauth tokens for deleted accounts,
+            // because distributed state).
+            if (body.code === 401 || body.errno === 102) {
+              logger.info('request.auth_server.fail', body);
+              return reject(new AppError.unauthorized(body.message));
+            }
+            // There should be no other 400-level errors, unless we're
+            // sending a badly-formed request of our own.  That warrants
+            // an "Internal Server Error" on our part.
+            logger.error('request.auth_server.fail', body);
+            return reject(new AppError({
+              code: 500,
+              message: 'error communicating with auth server'
+            }));
+          }
 
-      if (! body) {
-        return reply(
-          new AppError('empty body from auth response')
-        );
-      }
-      const result = {};
-      if (typeof body.email !== 'undefined') {
-        result.email = body.email;
-      }
-      if (typeof body.locale !== 'undefined') {
-        result.locale = body.locale;
-      }
-      // Translate from internal terminology into OAuth-style terminology.
-      if (typeof body.authenticationMethods !== 'undefined') {
-        result.amrValues = body.authenticationMethods;
-      }
-      if (typeof body.authenticatorAssuranceLevel !== 'undefined') {
-        result.twoFactorAuthentication = body.authenticatorAssuranceLevel >= 2;
-      }
-      reply(result);
-    });
+          if (! body) {
+            return reject(AppError('empty body from auth response'));
+          }
+          const result = {};
+          if (typeof body.email !== 'undefined') {
+            result.email = body.email;
+          }
+          if (typeof body.locale !== 'undefined') {
+            result.locale = body.locale;
+          }
+          // Translate from internal terminology into OAuth-style terminology.
+          if (typeof body.authenticationMethods !== 'undefined') {
+            result.amrValues = body.authenticationMethods;
+          }
+          if (typeof body.authenticatorAssuranceLevel !== 'undefined') {
+            result.twoFactorAuthentication = body.authenticatorAssuranceLevel >= 2;
+          }
+          return resolve(result);
+        });
+      });
+    }
+
+    return makeReq().then(result => result);
   }
 };
 

lib/routes/avatar/delete.js

@@ -31,41 +31,41 @@ module.exports = {
         .optional()
     }
   },
-  handler: function deleteAvatar(req, reply) {
+  handler: async function deleteAvatar(req) {
     if (req.params.id === DEFAULT_AVATAR_ID) {
       // if we are clearing the default avatar then do nothing
-      return reply({});
+      return {};
     }
 
     const uid = req.auth.credentials.user;
     let avatar, lookup;
 
-    req.server.methods.profileCache.drop(uid, () => {
-      if (req.params.id) {
-        lookup = getAvatar(req.params.id, uid);
-      } else {
-        lookup = getSelectedAvatar(uid);
-      }
-
-      return lookup.then(av => {
-        avatar = av;
-        return P.all([
-          db.deleteAvatar(avatar.id),
-          db.getProviderById(avatar.providerId)
-        ]);
-      })
-      .spread((_, provider) => {
-        logger.debug('provider', provider);
-        if (provider.name === FXA_PROVIDER) {
-          return workers.delete(avatar.id);
-        }
-      })
+    return req.server.methods.profileCache.drop(uid)
       .then(() => {
-        notifyProfileUpdated(uid); // Don't wait on promise
-        return EMPTY;
-      })
-      .done(reply, reply);
-    });
+        if (req.params.id) {
+          lookup = getAvatar(req.params.id, uid);
+        } else {
+          lookup = getSelectedAvatar(uid);
+        }
+
+        return lookup.then(av => {
+          avatar = av;
+          return P.all([
+            db.deleteAvatar(avatar.id),
+            db.getProviderById(avatar.providerId)
+          ]);
+        })
+          .spread((_, provider) => {
+            logger.debug('provider', provider);
+            if (provider.name === FXA_PROVIDER) {
+              return workers.delete(avatar.id);
+            }
+          })
+          .then(() => {
+            notifyProfileUpdated(uid); // Don't wait on promise
+            return EMPTY;
+          });
+      });
   }
 };