themetalfleece / neogma

Object-Graph-Mapping neo4j framework, Fully-typed with TypeScript, for easy and flexible node and relationship operations
https://themetalfleece.github.io/neogma/
MIT License
124 stars 12 forks source link

Neogma constructor doesn't throw a "connection failed" error, exits the process instead #60

Closed Ansis100 closed 1 year ago

Ansis100 commented 1 year ago

https://github.com/themetalfleece/neogma/blob/5911f46d3ec0c79171c849c549409602096b5e40/src/Neogma.ts#L33-L43

In the code above you can see the connection initialization in the Neogma class constructor. On connection failure the catch block does not throw an error. Instead it exits the entire process.

This might be okay for general usage but there are cases where you would like to catch the error and retry the connection or log the error to some external service and gracefully shut down the service.

themetalfleece commented 1 year ago

Hey there,

Thanks for opening this issue. You are correct, throwing an error would be preferred. I'll get on it soon.

themetalfleece commented 1 year ago

Released in v1.12.0. I've also added an async neogma.verifyConnectivity() method which also throws a NeogmaConnectivityError.

Documentation

Ansis100 commented 1 year ago

Just tested it and it works only if I await neogma.verifyConnectivity() after initializing the instance.

The Neogma instance doesn't return a promise and verifyConnectivity() is async so it's difficult to catch the error in the constructor before attempting any further operations with the instance.

themetalfleece commented 1 year ago

Hey,

Yes, the constructor can't be async, and the driver's verifyConnectivity method is async. So, the only way is to create a separate async method (like I've done). You can run and await it right after you create the neogma instance.

Did you have anything else in mind on how it should be implemented?

Ansis100 commented 1 year ago

Did you have anything else in mind on how it should be implemented?

Haha, not really. Just pointing it out. Might be worth mentioning this in the docs.

I'm guessing the only way to change that would be to move initialization out of the constructor to some kind of async method.

The current method works great, though, thank you for the quick fix!

themetalfleece commented 1 year ago

Yeah I can definitely improve the docs! Cheers 💪