Stop connection again when post-connect queries or prepares fail (#115)

When a post-connect query or prepare failed, mysql:start_link did
raise a badmatch exception, but the connection process did not
exit and remained connected to the database.
With the changes in this commit, the connection process will be
stopped before raising an exception.

src/mysql.erl

@@ -714,22 +714,31 @@ encode(Conn, Term) ->
 
 %% @doc Executes the given queries and prepares the given statements after a
 %% connection has been made.
+%%
+%% If any of the queries or prepares fails, the connection is closed and an
+%% exception is raised.
 -spec execute_after_connect(connection(), [iodata()], [{atom(), iodata()}])
     -> ok.
 execute_after_connect(Conn, Queries, Prepares) ->
-    lists:foreach(fun (Query) ->
-                      case query(Conn, Query) of
-                          ok -> ok;
-                          {ok, _} -> ok;
-                          {ok, _, _} -> ok
-                      end
-                  end,
-                  Queries),
-    lists:foreach(fun ({Name, Stmt}) ->
-                      {ok, Name} = prepare(Conn, Name, Stmt)
-                  end,
-                  Prepares),
-    ok.
+    try
+        lists:foreach(fun (Query) ->
+                          case query(Conn, Query) of
+                              ok -> ok;
+                              {ok, _} -> ok;
+                              {ok, _, _} -> ok
+                          end
+                      end,
+                      Queries),
+        lists:foreach(fun ({Name, Stmt}) ->
+                          {ok, Name} = prepare(Conn, Name, Stmt)
+                      end,
+                      Prepares),
+        ok
+    catch
+        ?EXCEPTION(Class, Reason, Stacktrace) ->
+            catch stop(Conn, ?default_connect_timeout),
+            erlang:raise(Class, Reason, ?GET_STACK(Stacktrace))
+    end.
 
 %% @doc Makes a gen_server call for a query (plain, parametrized or prepared),
 %% checks the reply and sometimes throws an exception when we need to jump out

test/mysql_change_user_tests.erl

@@ -29,7 +29,7 @@
 %% Ensure that the current user can be changed to another user
 %% when given correct credentials.
 correct_credentials_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2)),
     ?assert(is_current_user(Pid, ?user2)),
     mysql:stop(Pid),
@@ -38,7 +38,7 @@ correct_credentials_test() ->
 %% Ensure that change user fails when given incorrect credentials,
 %% and that the current user still works.
 incorrect_credentials_fail_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     TrapExit = erlang:process_flag(trap_exit, true),
     ?assertError({1045, <<"28000">>, <<"Access denied", _/binary>>},
                  mysql:change_user(Pid, ?user2, ?password1)),
@@ -51,7 +51,7 @@ incorrect_credentials_fail_test() ->
 %% Ensure that user variables are reset after a successful change user
 %% operation.
 reset_variables_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ok = mysql:query(Pid, <<"SET @foo=123">>),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2)),
     ?assert(is_current_user(Pid, ?user2)),
@@ -65,7 +65,7 @@ reset_variables_test() ->
 %% Ensure that temporary tables are reset after a successful change user
 %% operation.
 reset_temptables_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ok = mysql:query(Pid, <<"CREATE DATABASE IF NOT EXISTS otptest">>),
     ok = mysql:query(Pid, <<"CREATE TEMPORARY TABLE otptest.foo (bar INT)">>),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2)),
@@ -79,7 +79,7 @@ reset_temptables_test() ->
 
 %% Ensure that change user fails when inside an unmanaged transaction.
 fail_in_unmanaged_transaction_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ok = mysql:query(Pid, <<"BEGIN">>),
     ?assert(mysql:in_transaction(Pid)),
     ?assertError(change_user_in_transaction,
@@ -91,7 +91,7 @@ fail_in_unmanaged_transaction_test() ->
 
 %% Ensure that change user fails when inside a managed transaction.
 fail_in_managed_transaction_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ?assertError(change_user_in_transaction,
                  mysql:transaction(Pid,
                                    fun () -> mysql:change_user(Pid,
@@ -103,7 +103,7 @@ fail_in_managed_transaction_test() ->
     ok.
 
 with_db_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ok = mysql:query(Pid, <<"CREATE DATABASE IF NOT EXISTS otptest">>),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2, [{database, <<"otptest">>}])),
     ?assert(is_current_user(Pid, ?user2)),
@@ -116,7 +116,7 @@ with_db_test() ->
     ok.
 
 execute_queries_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2, [{queries, [<<"SET @foo=123">>]}])),
     ?assert(is_current_user(Pid, ?user2)),
     ?assertEqual({ok,
@@ -126,8 +126,19 @@ execute_queries_test() ->
     mysql:stop(Pid),
     ok.
 
+execute_queries_failure_test() ->
+    Pid = connect_db(?user1, ?password1),
+    erlang:process_flag(trap_exit, true),
+    ?assertError(_Reason, mysql:change_user(Pid, ?user2, ?password2, [{queries, [<<"foo">>]}])),
+    receive
+        {'EXIT', Pid, normal} -> ok
+    after 1000 ->
+        error(no_exit_message)
+    end,
+    erlang:process_flag(trap_exit, false).
+
 prepare_statements_test() ->
-    Pid = connect_db(?user1),
+    Pid = connect_db(?user1, ?password1),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2, [{prepare, [{foo, <<"SELECT ? AS foo">>}]}])),
     ?assert(is_current_user(Pid, ?user2)),
     ?assertEqual({ok,
@@ -137,9 +148,20 @@ prepare_statements_test() ->
     mysql:stop(Pid),
     ok.
 
+prepare_statements_failure_test() ->
+    Pid = connect_db(?user1, ?password1),
+    erlang:process_flag(trap_exit, true),
+    ?assertError(_Reason, mysql:change_user(Pid, ?user2, ?password2, [{prepare, [{foo, <<"foo">>}]}])),
+    receive
+        {'EXIT', Pid, normal} -> ok
+    after 1000 ->
+        error(no_exit_message)
+    end,
+    erlang:process_flag(trap_exit, false).
+
 
-connect_db(User) ->
-    {ok, Pid} = mysql:start_link([{user, User}, {password, ?password1},
+connect_db(User, Password) ->
+    {ok, Pid} = mysql:start_link([{user, User}, {password, Password},
                                   {log_warnings, false}]),
     Pid.
 

test/mysql_tests.erl

@@ -206,6 +206,27 @@ unix_socket_test() ->
                                   "release could not be determined.~n")
     end.
     
+connect_queries_failure_test() ->
+    process_flag(trap_exit, true),
+    ?assertError(_Reason, mysql:start_link([{user, ?user}, {password, ?password},
+                                            {queries, ["foo"]}])),
+    receive
+        {'EXIT', _Pid, normal} -> ok
+    after 1000 ->
+        error(no_exit_message)
+    end,
+    process_flag(trap_exit, false).
+
+connect_prepare_failure_test() ->
+    process_flag(trap_exit, true),
+    ?assertError(_Reason, mysql:start_link([{user, ?user}, {password, ?password},
+                                            {prepare, [{foo, "foo"}]}])),
+    receive
+        {'EXIT', _Pid, normal} -> ok
+    after 1000 ->
+        error(no_exit_message)
+    end,
+    process_flag(trap_exit, false).
 
 %% For R16B where sys:get_state/1 is not available.
 get_state(Process) ->