Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some notes about migration from hapi v16 to v17 #468

Open
fgalan opened this issue Nov 13, 2018 · 2 comments
Open

Some notes about migration from hapi v16 to v17 #468

fgalan opened this issue Nov 13, 2018 · 2 comments
Labels

Comments

@fgalan
Copy link
Member

fgalan commented Nov 13, 2018

Initially on PR #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 17.0.0 Release Notes hapijs/hapi#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
Copy link
Member Author

fgalan commented Nov 13, 2018

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.

@jmcanterafonseca
Copy link

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()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants