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

JDBC driver prints stack trace to stdout #1827

Closed
jjtt opened this issue Sep 19, 2024 · 2 comments · Fixed by #1880
Closed

JDBC driver prints stack trace to stdout #1827

jjtt opened this issue Sep 19, 2024 · 2 comments · Fixed by #1880
Labels
Milestone

Comments

@jjtt
Copy link

jjtt commented Sep 19, 2024

Describe the bug

On some exceptions the JDBC driver prints a whole stack trace to stdout here: https://github.com/ClickHouse/clickhouse-java/blob/main/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseClient.java#L963

This can mess up structured logging output for an application that uses the ClickHouse JDBC driver.

Steps to reproduce

  1. Connect to a db from an application
  2. Revoke the db credentials
  3. See stack trace in application stdout

Expected behaviour

Should use some common logging framework where the application can control logging from libraries. For example: SLF4J or logback.

Or throw an exception and let the application worry about it.

@jjtt jjtt added the bug label Sep 19, 2024
@chernser chernser added this to the 0.7.1 milestone Sep 23, 2024
@chernser chernser modified the milestone: 0.7.1 Oct 2, 2024
@Am-phi
Copy link
Contributor

Am-phi commented Oct 22, 2024

Hello @chernser ,
Unless you already did something I'd be glad to solve this simple issue.
My take would be to have the same behaviour as the client_v2 and simply return false.

Even though I don't see generally a good idea to silence an exception, I wonder if adding a log is useful as the fact of having the clickhouse instance, we are trying to reach, unavailable is already part of the goal of calling this method. What is your opinion on the matter?

@chernser
Copy link
Contributor

Good day, @Am-phi !
Thank you for suggesting the help - I will appreciate it!

What you are saying is true - we should always log exception or re-throw it.
returning false in this case is a correct thing - because it indicates that server is not reachable. And logging the exception is important for investigation later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants