call close handler with correct parameters

[StarpTech]

No description provided.

[mcollina]

Can you add or update the unit tests?

[StarpTech]

I removed the conditions. There is no case to test I think.

[mcollina]

I would rather not regress this. Can you add a case similae to the example you added in for fastify?

[StarpTech]

This test fails on the previous code.

[StarpTech]

I also fixed the example #55

[mcollina]

LGTM

[delvedor]

I'm not sure this is the best thing to do.
Why you should call close more than one time?
IMHO if a user calls close twice there are two options:

1. E_TOO_MANY_BEERS 🍻
2. there is some error in its logic.

I think that if a user calls close more than one time we should log a working and ignore the command or throw an exception.

Thoughts?

[mcollina]

I agree with you that this is not an easy to understand behavior, but changing it would be semver-major and I prefer to have it fixed it right now. It's not a big change.

[delvedor]

but changing it would be semver-major

Only if we throw an error. Log a warning and ignore the command is not, because the user has already called close and we are closing the application anyhow.

[mcollina]

Only if we throw an error. Log a warning and ignore the command is not, because the user has already called close and we are closing the application anyhow.

The user expected the callback to be called.

[StarpTech]

@delvedor In my opinion, it's fine since we are working everywhere with queues. If you can explain it well to the user then it's an easy task.

IMHO if a user calls close twice there are two options: ...

or they split its shutdown routines into several pieces?

[delvedor]

The user expected the callback to be called.

Ok, then we can call it instantly with an error, such as Closing procedure already started.

app.close(err => {
  console.log(err) // null
})

app.close(err => {
  console.log(err) // 'Closing procedure already started'
})

[delvedor]

or they split its shutdown routines into several pieces?

@StarpTech what does it mean? You want to close only a part of your application and sometimes after another part?
Why not use two instances of avvio then?

[mcollina]

The approach you describe in #56 (comment) is correct. However it's a semver-major change, and I would prefer to land this first as a minor/patch.

[StarpTech]

@delvedor just realized that close won't propagate errors so it's not a great tool for this. I also tend to #56 (comment) but also removing the queue mechanism from close completely.

Let's discuss this behavior after this PR. This PR will fix a bug in the close handler specification.

[delvedor]

LGTM

[delvedor]

The approach you describe in #56 (comment) is correct. However it's a semver-major change, and I would prefer to land this first as a minor/patch.

I see it more as a bugfix than a major change.