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

(examples) Do not throw in async flows #7

Closed
XVincentX opened this issue Jul 24, 2017 · 5 comments
Closed

(examples) Do not throw in async flows #7

XVincentX opened this issue Jul 24, 2017 · 5 comments

Comments

@XVincentX
Copy link

Hey,
I was just having a look to this plugin and, reading the example, I noticed this:

fastify.register(require('fastify-mongodb'), {
  url: 'mongodb://mongo/db'
}, err => {
  if (err) throw err
})

Throwing in in async flow is an antipattern, it will probably crash your entire process as nobody can catch that exception. While it's just an example, you might want to write a different code to handle the error.

P.S: The same goes in the listen function below.

@delvedor
Copy link
Member

Throw in that callback will crash your process :)
The error handling is demanded to the user, it is your choice how handle it.

@XVincentX
Copy link
Author

The error handling is demanded to the user, it is your choice how handle it.

I know, I was just thinking that's is wrong to put it there; a good console.error in case of any error would probably be better, but it's your call.

@delvedor
Copy link
Member

It definitely depends on your code, for example, you have a web service that needs mongodb, without it cannot work.
What you do is try to connect to mongo with this plugin and if you can't, throw an error.

Don't forget that the callback is executed at bootstrap time, so if it have to crash, it will do it when you open up the server, not at "runtime" :)

@XVincentX
Copy link
Author

XVincentX commented Jul 24, 2017

I disagree but ok, I got your point.

@mcollina
Copy link
Member

The correct approach is not to place any callback there. The default behavior in case of error is well defined (it will emit an 'error' event on the server). We just need to update the README.

Placing console.error(err) there are wrong in both cases, as those are fatal and not-recoverable errors for the demo script.

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

No branches or pull requests

3 participants