'No suitable driver found' message when connection fails with nonexistent database

[pivotal-chorus]

We are using Sequel to connect to Greenplum (Postgres) database, and the error handling works perfectly in development (when we use the gem from Bundler).

In development, running the same code gives:

Sequel.connect('jdbc:postgresql://servername:5432/blah?user=admin&password=secret', test: true)
Sequel::DatabaseConnectionError: Java::OrgPostgresqlUtil::PSQLException: FATAL: database "blah" does not exist

But in production mode (gems are bundled using Jetpack), we get an incorrect error message:

Sequel.connect('jdbc:postgresql://servername:5432/blah?user=admin&password=secret', test: true)
Sequel::DatabaseConnectionError: Java::JavaSql::SQLException: No suitable driver found for 'jdbc:postgresql://servername:5432/blah?user=admin&password=secret'

This seems to be related to #477, since the DriverManager's getConnection method is blowing up. (which is another kettle of fish)

However, the main problem is that the wrong error message is shown. In the JDBC adapter, we're trying to concatenate error messages but that just doesn't work. (cf https://github.com/jeremyevans/sequel/blob/master/lib/sequel/adapters/jdbc.rb#L240)

One possible fix is not catching the second error and just letting it bubble up.

[jeremyevans]

The problem with just bubbling up the second exception the is that if driver.new.connect raises an exception (e2), you are basically ignoring the main exception (e, the one raised by JavaSQL::DriverManager.getConnection). So while bubbling up may fix the incorrect error message in your case, my guess is it's just as likely to cause incorrect error messages for other people.

You say that "we're trying to concatenate error messages but that just doesn't work", could you explain why? I think the principle is sound, is there a reason it doesn't work in your particular case? I don't see concatenated error messages in your production mode example, but I'm not sure if that is because you just didn't include them, or maybe something else is chopping off exception messages at the first line feed.

When I try with a bad database, I get the error messages concatenated:

$ jruby -I lib bin/sequel -t jdbc:postgresql://foo
Sequel::DatabaseConnectionError: NativeException: java.sql.SQLException: No suitable driver found for jdbc:postgresql://foo
Sequel::DatabaseError: driver.new.connect returned nil: probably bad JDBC connection string
         make_new at /home/billg/sequel/lib/sequel/connection_pool.rb:97
         ...

It looks like the error message you get depends on the error in the connection string. The above example is with a bad host, but when I do it with a bad database I get:

$ jruby -I lib bin/sequel -t jdbc:postgresql://localhost/foo?user=postgres
Sequel::DatabaseConnectionError: NativeException: org.postgresql.util.PSQLException: FATAL: database "foo" does not exist
NativeException: org.postgresql.util.PSQLException: FATAL: database "foo" does not exist
         make_new at /home/billg/sequel/lib/sequel/connection_pool.rb:97
         ...

I can see not concatenating the messages if they are identical, and possibly reordering the concatenation if the second exception message is usually better than the first, but dropping the concatenation entirely if the error messages are different just results in the loss of possibly useful information, so I'd be against that.

[jeremyevans]

How about the following?:

index adad09e..ca0c1ed 100644
--- a/lib/sequel/adapters/jdbc.rb
+++ b/lib/sequel/adapters/jdbc.rb
@@ -237,8 +237,10 @@ module Sequel
               raise(Sequel::DatabaseError, 'driver.new.connect returned nil: probably bad JDBC connection string') unless c
               c
             rescue JavaSQL::SQLException, NativeException, StandardError => e2
-              e.message << "\n#{e2.class.name}: #{e2.message}"
-              raise e
+              unless e2.message == e.message
+                e2.message << "\n#{e.class.name}: #{e.message}"
+              end
+              raise e2
             end
           end
         end

[pivotal-chorus]

We aren't ever getting the messages concatenated. With edge Sequel, when we connect to a nonexistent database, we only get a single line error message. To illustrate, we added some puts statements in the rescues:

irb> Sequel.connect("jdbc:postgresql://servername:5432/nonexistent?user=admin&password=secret", test: true)
Error e raised:
java.sql.SQLException: No suitable driver found for jdbc:postgresql://servername:5432/nonexistent?user=admin&password=secret
Error e2 raised:
org.postgresql.util.PSQLException: FATAL: database "nonexistent" does not exist
Re-raising e:
java.sql.SQLException: No suitable driver found for jdbc:postgresql://servername:5432/nonexistent?user=admin&password=secret
Sequel::DatabaseConnectionError: Java::JavaSql::SQLException: No suitable driver found for jdbc:postgresql://servername:5432/nonexistent?user=admin&password=secret

For some context, our use case is that users can register database servers and credentials and manage them through a web interface, and we're passing the error message to the front end. So we really don't want to show users the message that no suitable driver was found, if the actual error was that the database didn't exist.

[pivotal-chorus]

Also, is it expected for DriverManager.getConnection to fail? We are seeing it fail in production (with gems bundled with Jetpack) but not in development (where it's loaded using bundler). This is the same behavior reported in #477.

[jeremyevans]

I'm not sure why you aren't getting the error messages concatenated. As my example showed, the concatenation is happening in my environment. Can you figure out why the concatenation isn't working for you?

DriverManager.getConnection can fail in certain cases, which is why Sequel has the fallback code it does. I think it may be a bug in JRuby, but it's been that way for a long time.

Did you try the patch I posted, which gives priority to the e2 exception?

[pivotal-chorus]

Thanks for the information, your patch works for us.

You're right about DriverManager getConnection, we found a JRuby bug report. http://jira.codehaus.org/browse/JRUBY-6372

We've looked a little at why the error messages aren't concatenated, beats me. We're on JRuby 1.7.0 using the jdbc-postgres gem. What's your environment? We're on jruby 1.7.0 and postgres-jdbc.

[jeremyevans]

I think I was testing on JRuby 1.6.8. Considering the exception changes in JRuby 1.7, it's possible the exception message concatenation is not working there. I'll see if I can figure out a work around for it, probably involving raising a new exception with the combined message instead of modifying the existing exception's message.