Skip to content
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

fix: cb is not a function at fallbackErrorHandler #5296

Closed
wants to merge 2 commits into from

Conversation

DzenDyn
Copy link

@DzenDyn DzenDyn commented Jan 31, 2024

This fixes error if cb is not a function at fallbackErrorHandler:
image

Tests:
image

Benchmarks:
image

Cheklist:

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows the [Developer's Certification of Origin](

Signed-off-by: Evgeny Nikiforov <DzenDyn@yandex.ru>
@mcollina
Copy link
Member

Thanks for opening a PR! Can you please add a unit test?

@DzenDyn
Copy link
Author

DzenDyn commented Jan 31, 2024

I'm sorry, but I'm afraid I'm not quite sure how to do this.
I have not yet found how to trigger this error and at what point there may not be a callback, but Sentry caught an unhandled exception and the commit I proposed should fix it.

@metcoder95
Copy link
Member

I'm starting to guess if your error handler is bad set, I couldn't reproduce it easily.
What I found instead, is that we are not validating properly the error handler when setting fastify.setErrorHandler in contrast to what we do at a route level, where indeed fails if a bad handler is passed.

TypeError: Cannot read properties of undefined (reading 'bind')
    at Object.setErrorHandler (/Users/metcoder/Documents/Workspace/MetCoder/Fastify_Core/fastify.js:847:71)
    at Object.<anonymous> (/Users/metcoder/Documents/Workspace/MetCoder/Fastify_Core/playground.js:5:9)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
    at Module.load (node:internal/modules/cjs/loader:1207:32)
    at Module._load (node:internal/modules/cjs/loader:1023:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
    at node:internal/main/run_main_module:28:49

@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 31, 2024

Yes, should throw an error when setting the value when starting the webserver and not when the webserver is running.

@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 14, 2024

Closed as it was superseeded by #5317

@Uzlopak Uzlopak closed this Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants