-
Notifications
You must be signed in to change notification settings - Fork 412
fix: Fixed error reporting from async Fastify handlers #3100
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the original errors.test.js
file. I moved it to this file because it seems to be testing errors from the Express compatibility modules (though, I'm not completely convinced of that).
await promise | ||
}) | ||
|
||
test('asynchronous handler errors', async (t) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test fails without the promise: true
lines added in the instrumentation.
087f8f1
to
d8fc5ef
Compare
For those following along: the "fix" in this PR is updating the instrumentation to indicate that the request handlers are possibly promises. But that fix has caused unexpected, difficult, changes that we are working to resolve. |
Thanks for the update. I’m currently running into this issue as well—do you have an estimated timeline for when a fix might be in place? @jsumners-nr |
We had to backlog this for a bit to work on a higher priority item. We'll be picking this one back up shortly. No current ETA, but hopefully soon. |
This PR resolves #3099.