fastify
Error handler settings fails when preceded by an awaited register
Prerequisites
- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the bug has not already been reported
Fastify version
3.29.0
Plugin version
No response
Node.js version
16.15.0
Operating system
macOS
Operating system version (i.e. 20.04, 11.3, 10)
12.3.1
Description
When I set an error handler on a Fastify instance after registering a plugin within an awaited statement, the error handler does not work as expected.
An example scenario: Let f be a Fastify instance.
- Add a
/get handler on instancefthat throws an error. - Register a plugin on instance
fwith any prefix (e.g.,/bar) with anawaitkeyword. - Set error handler (call it
h) on instancef.
When fetching / on this instance, an error is thrown and h is expected to run. However, it does not. The default handler is run instead.
Removing the await keyword fixes this issue. But the existence of the await keyword or the nonexistence thereof should not alter any behavior--unless there is an intricacy in this situation that forces such behavior.
Steps to Reproduce
A reproduction of the issue can be found here: https://github.com/omothm/fastify-error-handler-await-repro
Expected Behavior
All 3 test scenarios in the reproduction repo are expected to pass. Currently, one scenario fails (the one with await instance.register()) but was expected to pass.
This is... expected given some of the intricacies in the code. However I think this is solved in v4.0.0-rc.1. Can you check?
I've checked and, unfortunately, the behavior is the same in v4.0.0-rc.1.
This bug has cost me 2 days to figure out. I know it's on me, not the framework. But I thought pointing out this issue might help other developers.
I can't think of robust workarounds to make this unexpected behavior clear to the user. Perhaps the framework typing should remove the PromiseLike return value of instance.register()? This way, mistakenly awaiting the function would make the IDE throw a warning. (If there are use cases that require the PromiseLike interface, then this suggestion is obviously impractical.)
Or maybe put somewhere in the docs a concrete description of what to expect by awaiting/non-awaiting in conjunction with the effect of calls order (because awaiting after setting the error handler actually works)?
The await for .register is very useful. It can reduce the code like.
// without await
fastify.register(something).after(something)
// with await
await fastify.register(something)
// ...something depends on top
The problem of your issue is that, when you await the plugin. It will finalize the encapsulation process, so all the mutation on the parent instance will not be inherited.
IMHO, I only recommend to await the plugin which wrapped by fastify-plugin. In this case, it will not have the side-effect like you mention.
@climba03003 Thank you for your elaboration. How can we make it clear to the developer that awaiting a plugin finalizes encapsulation (and the consequences thereof)?
Perhaps adding a sentence or two here could prove useful. Do you have any recommended wording for the behavior that could be added there?
Hey there all 👋🏽 Long time user, first time contributor.
May I take this "good first issue" on?
My plan would be to put in a PR to update plugins.md#asyncawait with the guidance that @climba03003 shared above.
Hi @mcollina , can you assign me this one?
I would love to work on it.
Regards
can you assign me this one?
There is no need for assignment. If you would like to works on it. Just send the PR.
