-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Error handler settings fails when preceded by an awaited register #3863
Comments
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 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 // 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 IMHO, I only recommend to |
@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 👋🏽 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. |
go for it! |
Hi @mcollina , can you assign me this one? I would love to work on it. Regards |
There is no need for assignment. |
Closed by #4846 |
Prerequisites
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./
get handler on instancef
that throws an error.f
with any prefix (e.g.,/bar
) with anawait
keyword.h
) on instancef
.When fetching
/
on this instance, an error is thrown andh
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 theawait
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.The text was updated successfully, but these errors were encountered: