fix: Clean up hooks code

[vonagam]

Fixes two errors:

1. finally hooks can possibly run twice for same method call if one of them throws. Also a finally hook if throws activates error hooks, this is in conflict with this comment and also Promise.finally and try/catch/finally convention. Because currently it is like this (1, 2):

try {
  run([...before])();
  method();
  run([...after, ...finally])();
} catch {
  run([...error, ...finally])();
}

2. An error hook can get a context which is not up to date, if any before hook returns new context object and a service method throws, because errors (1, 2) thrown in a method invocation do no have hook property set (which should contain a last valid hook context), that's why error hooks end up using an initial hook object.

Added fixes and tests for such cases. Also small clean up changes for readability and performance.

A question for future: What are use cases for allowing returning new context object from hook functions where modifying context in place is not as convenient?

[vonagam]

What about adding finally to the list of hook types?

[daffl]

This makes a lot of sense, thank you! That the finally hooks are not documented is somewhat intentional because they were really only intended to be used internally (to send events). For the next major version I'd prefer to go in the direction the Koa style hook system outline in #932 which would allow a finally like this:

app.service('users').hooks({
  create: [
    async (context, next) => {
      const finally = async () => {
        // Finally code here
      }

      try {
        await next();
        await finally();  
      } catch (error) {
        await finally();
        throw error;
      }
    }
  ]
}

A convenience wrapper may also make this shorter.

[vonagam]

Cool about koa style, prefer it too.

By the way your example snippet has the same problem as this pull request solves for current system: if your finally() function throws it will run twice. Here is how it should be written:

Instead of:

try {
  await next();
  await finally();  
} catch (error) {
  await finally();
  throw error;
}

It should be:

try {
  await next();
} finally {
  await finally();
}

[daffl]

Ah, totally. That's totally it.