fiware-sth-comet icon indicating copy to clipboard operation
fiware-sth-comet copied to clipboard

Some notes about migration from hapi v16 to v17

Open fgalan opened this issue 6 years ago • 2 comments

Initially on PR https://github.com/telefonicaid/fiware-sth-comet/pull/467 I tried to use upgrade to hapi 17 to solve a vulnerability in the old version but at the end it was too hard, so I rolled back to use 16 (fortunatelly, a newer minor of 16 solved also the vulnerability).

However, I get some useful findings that could be useful in the future if this work needs to be done a the end (e.g. an new vulnerability wihtout fix in 16 minor):

  • Change server.on('log', function (event, tags), {...}) to server.events.on('log', function (event, tags) { ... }
  • Change server.on('request-internal', function (request, event, tags) { ... } to server.events.on({ name: 'request', channels: 'internal' }, function(request, event, tags) { ... }.
  • Don't use server.connection(options) function. Instead, use the options object as parameter in the hapi.Server constructor.
  • server.start() doesn't use a callback as parameter any longer. Notes at https://github.com/hapijs/hapi/issues/3658 shows the new patter, based in await. However, I haven't been able to adapt the code using this.

The list is maybe incomplete (I stalled at the last one before giving up)

fgalan avatar Nov 13 '18 12:11 fgalan

This was my attemp to addapt server.start()

  async function startServer() {

    // Start the server
    try {
      await server.start();
      return null;
    }
    catch (err) {
      return err;
    }
  }

  startServer().then(function(err) {
    callback(err, server);
  });

However, unit test were failing.

fgalan avatar Nov 13 '18 12:11 fgalan

see example at

https://github.com/FIWARE/NGSI-LD_TestSuite/blob/master/notifications/accumulator.js#L19

either you do

server.start.then() or await server.start()

jmcanterafonseca avatar Mar 26 '19 15:03 jmcanterafonseca