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

Test Connection (with error reason) #4959

Closed
SamJakob opened this issue Nov 25, 2023 · 5 comments · Fixed by #4961
Closed

Test Connection (with error reason) #4959

SamJakob opened this issue Nov 25, 2023 · 5 comments · Fixed by #4961
Labels
enhancement New feature or request

Comments

@SamJakob
Copy link

Is your feature request related to a problem? Please describe.
I know isConnected() exists to check whether the database driver is connected to the database, but is there some universal way of determining why the database is not connected if this is false?

For instance, if an invalid database is selected, isConnected just returns false but as isConnected could be false for many reasons it would be very helpful (e.g., to better aid with debugging) to have some way of retrieving the cause for the connection failure.

Describe the solution you'd like
Perhaps for backwards compatibility and simplicity, add a new method for this feature - testConnection() that could return:

  • a failure reason string or object on error and null on success
  • or, alternatively, a single object of a form similar to { isConnected: boolean; reason?: string; }

I'm not sure what other metadata might be useful to include, but at the very least a reason string would be helpful.

This might have to be implemented at the driver level.

Describe alternatives you've considered

  • Not bothering to test at all - this allows getting the actual reason for a failure when it happens in the form of an exception - but in some applications, it might not be immediately obvious that the database connection has failed and this could cause problems later on as users try to interact with the application.
  • Checking if isConnected() is false and then issuing a test query to the database to determine the reason. The issue with this is that there does not seem to be one universal query across all database types that is valid and which does not make any modifications, see: https://stackoverflow.com/questions/3668506/efficient-sql-test-query-or-validation-query-that-will-work-across-all-or-most (and that's not even mentioning MongoDB too).
@SamJakob SamJakob added the enhancement New feature or request label Nov 25, 2023
@B4nan
Copy link
Member

B4nan commented Nov 25, 2023

What the isConnected method does is doing a select 1 query and returns false if there is an exception. Surely we can have another method which will give you the exception back.

@SamJakob
Copy link
Author

SamJakob commented Nov 25, 2023

I assume it's different for Mongo though? And some databases might not support SELECT 1 particularly if more are supported in future?

In any case - yes - I would say it would definitely be worth having the additional method.

EDIT: and yes, I suppose it would be simple enough to return the value from exception using one of the two approaches above.

However, if it's possible to get that error message back without an exception being involved that would perhaps be better as it would mean we don't have the added cost of throwing and catching when the method is intended to check for errors.

@B4nan
Copy link
Member

B4nan commented Nov 25, 2023

There is no way to detect a broken connection, you need to do a query and catch the error. And this wont work with mongo, if that's you case, they changed some internals with the last major I think and removed the way to detect live connection completely. We could try the same approach with a random query maybe.

@SamJakob
Copy link
Author

Of course - I do get that a query of some kind needs to be run and then an error handled, I just wasn't sure of there was enough control over the lower level driver stuff to handle the error without exceptions (it's really of a micro-optimization honestly) - as I personally prefer to use exceptions only in exceptional circumstances as a matter of efficiency.

Anyway, that aside, perhaps it's worth having logic in the core to execute a test query and then driver-specific code to determine what that query is? That would hopefully give the flexibility to implement a solution for Mongo too.

Mongo's implementation could maybe use the ping command: https://www.mongodb.com/docs/manual/reference/command/ping/

or fetch DB stats: https://stackoverflow.com/a/7984054

@B4nan
Copy link
Member

B4nan commented Nov 25, 2023

The ping could be a good candidate for this.

B4nan added a commit that referenced this issue Nov 26, 2023
B4nan added a commit that referenced this issue Nov 26, 2023
B4nan added a commit that referenced this issue Nov 26, 2023
```ts
// boolean
const isConnected = await orm.isConnected();
// object with `ok`, `reason` and `error` keys
const check = await orm.checkConnection();

console.log(check.ok, check.reason);
```

Closes #4959
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants