From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 18 Nov 2012 22:32:57 +0000 (+0100)
Subject: Rewrite 461c7168 solving Oracle silent connection failures
X-Git-Tag: v0.08205~103
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=691583dfab7c7d633a1daac8fba8bd1a1832429b

Rewrite 461c7168 solving Oracle silent connection failures

The patch incorrectly masks the underlying issue - during the refactor
of 584ea6e4 a bunch of exception traps were introduced which were never
there in the first place. _dbh_get_info() should always be throwing an
error if one takes place (the only place before 584ea6e4 was in the
FreeTDS version determination, and that one made no sense anyway).
_server_info() OTOH is indeed an opportunistic routine - leave the
"rethrow if determining driver" escape as-is
---

diff --git a/TODO_SHORTTERM b/TODO_SHORTTERM
index b4a6899..ca4fc83 100644
--- a/TODO_SHORTTERM
+++ b/TODO_SHORTTERM
@@ -5,7 +5,5 @@ threads.pm in a BEGIN block?!?!?!
 bullshit
 * a0361822 reenable the win32 test - if it no longer fails for anyone - how
 will we ever fix it?
-* 461c7168 has two places where it sets a brand new lexical to undef. Please
-remove for the sanity of future maintainers (avoid the "why is this here?!")
 * a48693f4 adds 5 files for a test that may even be the same as that from
 571df676 - please rewrite using the existing schema and delete the rest
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 3af0805..91c4118 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1078,15 +1078,15 @@ sub _server_info {
 
     $info = {};
 
-    my $server_version;
-    try {
-      $server_version = $self->_get_server_version;
-    }
-    catch {
-      if ($self->{_in_determine_driver}) {
-        $self->throw_exception($_);
-      }
-      $server_version = undef;
+    my $server_version = try {
+      $self->_get_server_version
+    } catch {
+      # driver determination *may* use this codepath
+      # in which case we must rethrow
+      $self->throw_exception($_) if $self->{_in_determine_driver};
+
+      # $server_version on failure
+      undef;
     };
 
     if (defined $server_version) {
@@ -1131,19 +1131,7 @@ sub _dbh_get_info {
       unless defined $info;
   }
 
-  my $res;
-
-  try {
-    $res = $self->_get_dbh->get_info($info);
-  }
-  catch {
-    if ($self->{_in_determine_driver}) {
-      $self->throw_exception($_);
-    }
-    $res = undef;
-  };
-
-  return $res;
+  return $self->_get_dbh->get_info($info);
 }
 
 sub _describe_connection {
diff --git a/t/73oracle.t b/t/73oracle.t
index 6433ce6..16e3f63 100644
--- a/t/73oracle.t
+++ b/t/73oracle.t
@@ -91,6 +91,13 @@ is (
   'insert returning capability guessed correctly'
 );
 
+isa_ok (DBICTest::Schema->connect($dsn, $user, $pass)->storage->sql_maker, 'DBIx::Class::SQLMaker::Oracle');
+
+# see if determining a driver with bad credentials throws propely
+throws_ok {
+  DBICTest::Schema->connect($dsn, "BORKED BORKED USER $user", $pass)->storage->sql_maker;
+} qr/DBI Connection failed/;
+
 ##########
 # the recyclebin (new for 10g) sometimes comes in the way
 my $on_connect_sql = $v >= 10 ? ["ALTER SESSION SET recyclebin = OFF"] : [];