Browse Source

Merge pull request #277 from keynslug/otp-25-compat

Support blessed `format_status/1` callback
Sergey Prokhorov 2 years ago
parent
commit
ebc262b782
4 changed files with 85 additions and 8 deletions
  1. 2 2
      .github/workflows/ci.yml
  2. 27 2
      src/epgsql_sock.erl
  3. 44 1
      test/epgsql_SUITE.erl
  4. 12 3
      test/epgsql_ct.erl

+ 2 - 2
.github/workflows/ci.yml

@@ -53,7 +53,7 @@ jobs:
     env:
     env:
       SHELL: /bin/sh            # needed for erlexec
       SHELL: /bin/sh            # needed for erlexec
     steps:
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
 
       - uses: erlef/setup-beam@v1
       - uses: erlef/setup-beam@v1
         with:
         with:
@@ -70,7 +70,7 @@ jobs:
         run: PATH=$PATH:/usr/lib/postgresql/${{matrix.pg}}/bin/ make test
         run: PATH=$PATH:/usr/lib/postgresql/${{matrix.pg}}/bin/ make test
 
 
       - name: Upload CT logs artifact
       - name: Upload CT logs artifact
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v3
         if: failure()
         if: failure()
         with:
         with:
           name: ct_report_pg-${{matrix.pg}}_otp-${{matrix.otp}}
           name: ct_report_pg-${{matrix.pg}}_otp-${{matrix.otp}}

+ 27 - 2
src/epgsql_sock.erl

@@ -56,7 +56,7 @@
          get_backend_pid/1,
          get_backend_pid/1,
          activate/1]).
          activate/1]).
 
 
--export([handle_call/3, handle_cast/2, handle_info/2, format_status/2]).
+-export([handle_call/3, handle_cast/2, handle_info/2, format_status/1, format_status/2]).
 -export([init/1, code_change/3, terminate/2]).
 -export([init/1, code_change/3, terminate/2]).
 
 
 %% loop callback
 %% loop callback
@@ -68,6 +68,10 @@
          get_parameter_internal/2,
          get_parameter_internal/2,
          get_subproto_state/1, set_packet_handler/2]).
          get_subproto_state/1, set_packet_handler/2]).
 
 
+-ifdef(TEST).
+-export([state_to_map/1]).
+-endif.
+
 -export_type([transport/0, pg_sock/0, error/0]).
 -export_type([transport/0, pg_sock/0, error/0]).
 
 
 -include("protocol.hrl").
 -include("protocol.hrl").
@@ -318,12 +322,22 @@ terminate(_Reason, #state{mod = ssl, sock = Sock}) -> ssl:close(Sock).
 code_change(_OldVsn, State, _Extra) ->
 code_change(_OldVsn, State, _Extra) ->
     {ok, State}.
     {ok, State}.
 
 
+format_status(Status = #{reason := _Reason, state := State}) ->
+  %% Do not format the rows attribute when process terminates abnormally
+  %% but allow it when is a sys:get_status/1.2
+  Status#{state => redact_state(State)};
+format_status(Status) ->
+    Status.
+
+%% TODO
+%% This is deprecated since OTP-25 in favor of `format_status/1`. Remove once
+%% OTP-25 becomes minimum supported OTP version.
 format_status(normal, [_PDict, State=#state{}]) ->
 format_status(normal, [_PDict, State=#state{}]) ->
   [{data, [{"State", State}]}];
   [{data, [{"State", State}]}];
 format_status(terminate, [_PDict, State]) ->
 format_status(terminate, [_PDict, State]) ->
   %% Do not format the rows attribute when process terminates abnormally
   %% Do not format the rows attribute when process terminates abnormally
   %% but allow it when is a sys:get_status/1.2
   %% but allow it when is a sys:get_status/1.2
-  State#state{rows = information_redacted}.
+  redact_state(State).
 
 
 %% -- internal functions --
 %% -- internal functions --
 -spec handle_socket_pasive(pg_sock()) -> pg_sock().
 -spec handle_socket_pasive(pg_sock()) -> pg_sock().
@@ -785,3 +799,14 @@ handle_xlog_data(StartLSN, EndLSN, WALRecord,
               last_flushed_lsn = LastFlushedLSN,
               last_flushed_lsn = LastFlushedLSN,
               last_applied_lsn = LastAppliedLSN,
               last_applied_lsn = LastAppliedLSN,
               cbstate = NewCbState}.
               cbstate = NewCbState}.
+
+redact_state(State) ->
+    State#state{rows = information_redacted}.
+
+-ifdef(TEST).
+
+state_to_map(State) ->
+    [state | Fields] = tuple_to_list(State),
+    maps:from_list(lists:zip(record_info(fields, state), Fields)).
+
+-endif.

+ 44 - 1
test/epgsql_SUITE.erl

@@ -15,6 +15,9 @@
     end_per_suite/1
     end_per_suite/1
 ]).
 ]).
 
 
+%% logger handler
+-export([log/2]).
+
 -compile([export_all, nowarn_export_all]).
 -compile([export_all, nowarn_export_all]).
 
 
 modules() ->
 modules() ->
@@ -83,7 +86,8 @@ groups() ->
         ]},
         ]},
         {generic, [parallel], [
         {generic, [parallel], [
             with_transaction,
             with_transaction,
-            mixed_api
+            mixed_api,
+            redacted_state
         ]}
         ]}
     ],
     ],
 
 
@@ -1674,6 +1678,45 @@ recv_incremental_active_n(C, Ref, NumPassive, Rows, Others) ->
             error({timeout, NumPassive, Others, Rows})
             error({timeout, NumPassive, Others, Rows})
     end.
     end.
 
 
+redacted_state(Config) ->
+    case erlang:system_info(otp_release) of
+      V = [_, _] when V > "20" ->
+        redacted_state_(Config);
+      V ->
+        {skip, {"Logger facility is available starting OTP 21, running on OTP " ++ V}}
+    end.
+
+redacted_state_(Config) ->
+    _Handle = ct:timetrap({seconds, 3}),
+    try
+      logger:add_handler(?MODULE, ?MODULE, #{relay_to => self()}),
+      C = epgsql_ct:connect(Config),
+      true = unlink(C),
+      Reason = {please, ?MODULE, ?FUNCTION_NAME},
+      ok = proc_lib:stop(C, Reason, 1000),
+      receive
+        {log, Message} ->
+            ?assertMatch({report, #{label := {gen_server, terminate},
+                                    reason := Reason,
+                                    state := _}},
+                         Message),
+            {report, #{state := State}} = Message,
+            ?assertMatch(#{rows := information_redacted},
+                         epgsql_sock:state_to_map(State))
+      end
+    after
+      logger:remove_handler(?MODULE)
+    end.
+
+%% =============================================================================
+%% Logger handler
+%% ============================================================================
+
+log(#{msg := Msg}, #{relay_to := Pid}) ->
+    Pid ! {log, Msg};
+log(_, _) ->
+    ok.
+
 %% =============================================================================
 %% =============================================================================
 %% Internal functions
 %% Internal functions
 %% ============================================================================
 %% ============================================================================

+ 12 - 3
test/epgsql_ct.erl

@@ -4,6 +4,7 @@
 
 
 -export([
 -export([
     connection_data/1,
     connection_data/1,
+    connect/1,
     connect_only/2,
     connect_only/2,
     with_connection/2,
     with_connection/2,
     with_connection/3,
     with_connection/3,
@@ -19,6 +20,16 @@ connection_data(Config) ->
     Port = ?config(port, PgConfig),
     Port = ?config(port, PgConfig),
     {Host, Port}.
     {Host, Port}.
 
 
+connect(Config) ->
+    connect(Config, "epgsql_test", []).
+
+connect(Config, Username, Args) ->
+    {Host, Port} = connection_data(Config),
+    Module = ?config(module, Config),
+    Args2 = [{port, Port}, {database, "epgsql_test_db1"} | Args],
+    {ok, C} = Module:connect(Host, Username, Args2),
+    C.
+
 connect_only(Config, Args) ->
 connect_only(Config, Args) ->
     {Host, Port} = connection_data(Config),
     {Host, Port} = connection_data(Config),
     Module = ?config(module, Config),
     Module = ?config(module, Config),
@@ -39,10 +50,8 @@ with_connection(Config, F, Args) ->
     with_connection(Config, F, "epgsql_test", Args).
     with_connection(Config, F, "epgsql_test", Args).
 
 
 with_connection(Config, F, Username, Args) ->
 with_connection(Config, F, Username, Args) ->
-    {Host, Port} = connection_data(Config),
     Module = ?config(module, Config),
     Module = ?config(module, Config),
-    Args2 = [{port, Port}, {database, "epgsql_test_db1"} | Args],
-    {ok, C} = Module:connect(Host, Username, Args2),
+    C = connect(Config, Username, Args),
     try
     try
         F(C)
         F(C)
     after
     after