Skip to content

Commit 1256bd1

Browse files
GSMLG-BOTclaude
andcommitted
style: Fix all Credo readability issues
Fixed all 4 Credo readability issues by improving code style and formatting. ## Changes ### 1. Add @moduledoc to TestProcess (test/support/test_process.ex) - Added comprehensive module documentation - Explains purpose as test helper module - Documents usage for session process testing ### 2. Use Implicit Try (lib/phoenix/session_process/process_superviser.ex) - Refactored do_call_on_session/6 to use implicit try - Refactored do_cast_on_session/5 to use implicit try - Moved catch clauses outside of explicit try blocks - Improved code readability per Elixir best practices ### 3. Fix Long Lines (lib/phoenix/session_process/cleanup.ex) - Broke long line into multiple lines (was 151 chars, max 120) - Improved readability of emit_auto_cleanup_event call ### 4. Format Code (mix.exs, lib/phoenix/session_process/telemetry_logger.ex) - Applied Elixir standard formatting - Removed unnecessary quotes from keyword list atoms - Broke long function calls into multiple lines - Improved overall code consistency ## Quality Metrics - Credo readability issues: 4 → 0 ✅ - All 65 tests passing - Zero compilation warnings - Only 30 cosmetic design suggestions remain Co-Authored-By: Claude <[email protected]>
1 parent e6b5a1f commit 1256bd1

File tree

5 files changed

+173
-86
lines changed

5 files changed

+173
-86
lines changed

lib/phoenix/session_process/cleanup.ex

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,13 @@ defmodule Phoenix.SessionProcess.Cleanup do
9898
def handle_info({:cleanup_session, session_id}, state) do
9999
if Phoenix.SessionProcess.ProcessSupervisor.session_process_started?(session_id) do
100100
session_pid = Phoenix.SessionProcess.ProcessSupervisor.session_process_pid(session_id)
101-
Phoenix.SessionProcess.Telemetry.emit_auto_cleanup_event(session_id, Phoenix.SessionProcess.Helpers.get_session_module(session_pid), session_pid)
101+
102+
Phoenix.SessionProcess.Telemetry.emit_auto_cleanup_event(
103+
session_id,
104+
Phoenix.SessionProcess.Helpers.get_session_module(session_pid),
105+
session_pid
106+
)
107+
102108
Phoenix.SessionProcess.terminate(session_id)
103109
end
104110

lib/phoenix/session_process/process_superviser.ex

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -306,40 +306,36 @@ defmodule Phoenix.SessionProcess.ProcessSupervisor do
306306
end
307307

308308
defp do_call_on_session(session_id, pid, module, request, timeout, start_time) do
309-
try do
310-
result = GenServer.call(pid, request, timeout)
309+
result = GenServer.call(pid, request, timeout)
310+
duration = System.monotonic_time() - start_time
311+
Telemetry.emit_session_call(session_id, module, pid, request, duration: duration)
312+
result
313+
catch
314+
:exit, {:timeout, _} ->
311315
duration = System.monotonic_time() - start_time
312-
Telemetry.emit_session_call(session_id, module, pid, request, duration: duration)
313-
result
314-
catch
315-
:exit, {:timeout, _} ->
316-
duration = System.monotonic_time() - start_time
317316

318-
Telemetry.emit_communication_error(session_id, module, :call, :timeout,
319-
duration: duration
320-
)
317+
Telemetry.emit_communication_error(session_id, module, :call, :timeout,
318+
duration: duration
319+
)
321320

322-
Error.timeout(timeout)
321+
Error.timeout(timeout)
323322

324-
:exit, reason ->
325-
duration = System.monotonic_time() - start_time
326-
Telemetry.emit_communication_error(session_id, module, :call, reason, duration: duration)
327-
Error.call_failed(module, :call, {request}, reason)
328-
end
323+
:exit, reason ->
324+
duration = System.monotonic_time() - start_time
325+
Telemetry.emit_communication_error(session_id, module, :call, reason, duration: duration)
326+
Error.call_failed(module, :call, {request}, reason)
329327
end
330328

331329
defp do_cast_on_session(session_id, pid, module, request, start_time) do
332-
try do
333-
result = GenServer.cast(pid, request)
330+
result = GenServer.cast(pid, request)
331+
duration = System.monotonic_time() - start_time
332+
Telemetry.emit_session_cast(session_id, module, pid, request, duration: duration)
333+
result
334+
catch
335+
:exit, reason ->
334336
duration = System.monotonic_time() - start_time
335-
Telemetry.emit_session_cast(session_id, module, pid, request, duration: duration)
336-
result
337-
catch
338-
:exit, reason ->
339-
duration = System.monotonic_time() - start_time
340-
Telemetry.emit_communication_error(session_id, module, :cast, reason, duration: duration)
341-
Error.cast_failed(module, :cast, {request}, reason)
342-
end
337+
Telemetry.emit_communication_error(session_id, module, :cast, reason, duration: duration)
338+
Error.cast_failed(module, :cast, {request}, reason)
343339
end
344340

345341
@spec child_name(binary()) :: {:via, Registry, {Phoenix.SessionProcess.Registry, binary()}}

lib/phoenix/session_process/telemetry_logger.ex

Lines changed: 133 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,24 @@ defmodule Phoenix.SessionProcess.TelemetryLogger do
4949
def attach_default_logger(opts \\ []) do
5050
level = Keyword.get(opts, :level, :info)
5151

52-
:telemetry.attach_many("phoenix-session-process-default-logger", [
53-
[:phoenix, :session_process, :start],
54-
[:phoenix, :session_process, :stop],
55-
[:phoenix, :session_process, :start_error],
56-
[:phoenix, :session_process, :communication_error],
57-
[:phoenix, :session_process, :call],
58-
[:phoenix, :session_process, :cast],
59-
[:phoenix, :session_process, :auto_cleanup],
60-
[:phoenix, :session_process, :cleanup],
61-
[:phoenix, :session_process, :cleanup_error]
62-
], fn event, measurements, metadata, _config ->
63-
handle_default_event(event, measurements, metadata, level)
64-
end, %{level: level})
52+
:telemetry.attach_many(
53+
"phoenix-session-process-default-logger",
54+
[
55+
[:phoenix, :session_process, :start],
56+
[:phoenix, :session_process, :stop],
57+
[:phoenix, :session_process, :start_error],
58+
[:phoenix, :session_process, :communication_error],
59+
[:phoenix, :session_process, :call],
60+
[:phoenix, :session_process, :cast],
61+
[:phoenix, :session_process, :auto_cleanup],
62+
[:phoenix, :session_process, :cleanup],
63+
[:phoenix, :session_process, :cleanup_error]
64+
],
65+
fn event, measurements, metadata, _config ->
66+
handle_default_event(event, measurements, metadata, level)
67+
end,
68+
%{level: level}
69+
)
6570
end
6671

6772
@doc """
@@ -71,12 +76,17 @@ defmodule Phoenix.SessionProcess.TelemetryLogger do
7176
def attach_worker_events(opts \\ []) do
7277
level = Keyword.get(opts, :level, :debug)
7378

74-
:telemetry.attach_many("phoenix-session-process-worker-logger", [
75-
[:phoenix, :session_process, :worker_start],
76-
[:phoenix, :session_process, :worker_terminate]
77-
], fn event, measurements, metadata, _config ->
78-
handle_worker_event(event, measurements, metadata, level)
79-
end, %{level: level})
79+
:telemetry.attach_many(
80+
"phoenix-session-process-worker-logger",
81+
[
82+
[:phoenix, :session_process, :worker_start],
83+
[:phoenix, :session_process, :worker_terminate]
84+
],
85+
fn event, measurements, metadata, _config ->
86+
handle_worker_event(event, measurements, metadata, level)
87+
end,
88+
%{level: level}
89+
)
8090
end
8191

8292
@doc """
@@ -86,12 +96,17 @@ defmodule Phoenix.SessionProcess.TelemetryLogger do
8696
def attach_session_events(opts \\ []) do
8797
level = Keyword.get(opts, :level, :info)
8898

89-
:telemetry.attach_many("phoenix-session-process-session-logger", [
90-
[:phoenix, :session_process, :start],
91-
[:phoenix, :session_process, :stop]
92-
], fn event, measurements, metadata, _config ->
93-
handle_session_event(event, measurements, metadata, level)
94-
end, %{level: level})
99+
:telemetry.attach_many(
100+
"phoenix-session-process-session-logger",
101+
[
102+
[:phoenix, :session_process, :start],
103+
[:phoenix, :session_process, :stop]
104+
],
105+
fn event, measurements, metadata, _config ->
106+
handle_session_event(event, measurements, metadata, level)
107+
end,
108+
%{level: level}
109+
)
95110
end
96111

97112
@doc """
@@ -101,16 +116,21 @@ defmodule Phoenix.SessionProcess.TelemetryLogger do
101116
def attach_communication_events(opts \\ []) do
102117
level = Keyword.get(opts, :level, :info)
103118

104-
:telemetry.attach_many("phoenix-session-process-communication-logger", [
105-
[:phoenix, :session_process, :start],
106-
[:phoenix, :session_process, :stop],
107-
[:phoenix, :session_process, :call],
108-
[:phoenix, :session_process, :cast],
109-
[:phoenix, :session_process, :start_error],
110-
[:phoenix, :session_process, :communication_error]
111-
], fn event, measurements, metadata, _config ->
112-
handle_communication_event(event, measurements, metadata, level)
113-
end, %{level: level})
119+
:telemetry.attach_many(
120+
"phoenix-session-process-communication-logger",
121+
[
122+
[:phoenix, :session_process, :start],
123+
[:phoenix, :session_process, :stop],
124+
[:phoenix, :session_process, :call],
125+
[:phoenix, :session_process, :cast],
126+
[:phoenix, :session_process, :start_error],
127+
[:phoenix, :session_process, :communication_error]
128+
],
129+
fn event, measurements, metadata, _config ->
130+
handle_communication_event(event, measurements, metadata, level)
131+
end,
132+
%{level: level}
133+
)
114134
end
115135

116136
@doc """
@@ -120,13 +140,18 @@ defmodule Phoenix.SessionProcess.TelemetryLogger do
120140
def attach_cleanup_events(opts \\ []) do
121141
level = Keyword.get(opts, :level, :debug)
122142

123-
:telemetry.attach_many("phoenix-session-process-cleanup-logger", [
124-
[:phoenix, :session_process, :auto_cleanup],
125-
[:phoenix, :session_process, :cleanup],
126-
[:phoenix, :session_process, :cleanup_error]
127-
], fn event, measurements, metadata, _config ->
128-
handle_cleanup_event(event, measurements, metadata, level)
129-
end, %{level: level})
143+
:telemetry.attach_many(
144+
"phoenix-session-process-cleanup-logger",
145+
[
146+
[:phoenix, :session_process, :auto_cleanup],
147+
[:phoenix, :session_process, :cleanup],
148+
[:phoenix, :session_process, :cleanup_error]
149+
],
150+
fn event, measurements, metadata, _config ->
151+
handle_cleanup_event(event, measurements, metadata, level)
152+
end,
153+
%{level: level}
154+
)
130155
end
131156

132157
@doc """
@@ -201,81 +226,136 @@ defmodule Phoenix.SessionProcess.TelemetryLogger do
201226

202227
defp log_event(_event, _metadata), do: :ok
203228

204-
defp handle_worker_event([:phoenix, :session_process, :worker_start], _measurements, metadata, level) do
229+
defp handle_worker_event(
230+
[:phoenix, :session_process, :worker_start],
231+
_measurements,
232+
metadata,
233+
level
234+
) do
205235
if should_log?(level, metadata) do
206236
worker_spec = Map.get(metadata, :worker_spec, "unknown")
207237
Logger.debug("Worker start: #{worker_spec}")
208238
end
209239
end
210240

211-
defp handle_worker_event([:phoenix, :session_process, :worker_terminate], _measurements, metadata, level) do
241+
defp handle_worker_event(
242+
[:phoenix, :session_process, :worker_terminate],
243+
_measurements,
244+
metadata,
245+
level
246+
) do
212247
if should_log?(level, metadata) do
213248
pid = Map.get(metadata, :pid, "unknown")
214249
Logger.debug("Worker terminate: #{inspect(pid)}")
215250
end
216251
end
217252

218-
defp handle_session_event([:phoenix, :session_process, :session_start], _measurements, metadata, level) do
253+
defp handle_session_event(
254+
[:phoenix, :session_process, :session_start],
255+
_measurements,
256+
metadata,
257+
level
258+
) do
219259
if should_log?(level, metadata) do
220260
session_id = Map.get(metadata, :session_id, "unknown")
221261
Logger.info("Session start: #{session_id}")
222262
end
223263
end
224264

225-
defp handle_session_event([:phoenix, :session_process, :session_end], _measurements, metadata, level) do
265+
defp handle_session_event(
266+
[:phoenix, :session_process, :session_end],
267+
_measurements,
268+
metadata,
269+
level
270+
) do
226271
if should_log?(level, metadata) do
227272
session_id = Map.get(metadata, :session_id, "unknown")
228273
Logger.info("Session end: #{session_id}")
229274
end
230275
end
231276

232-
defp handle_communication_event([:phoenix, :session_process, :call], _measurements, metadata, level) do
277+
defp handle_communication_event(
278+
[:phoenix, :session_process, :call],
279+
_measurements,
280+
metadata,
281+
level
282+
) do
233283
if should_log?(level, metadata) do
234284
session_id = Map.get(metadata, :session_id, "unknown")
235285
message = Map.get(metadata, :message, "unknown")
236286
Logger.info("Session call #{session_id}: #{inspect(message)}")
237287
end
238288
end
239289

240-
defp handle_communication_event([:phoenix, :session_process, :cast], _measurements, metadata, level) do
290+
defp handle_communication_event(
291+
[:phoenix, :session_process, :cast],
292+
_measurements,
293+
metadata,
294+
level
295+
) do
241296
if should_log?(level, metadata) do
242297
session_id = Map.get(metadata, :session_id, "unknown")
243298
message = Map.get(metadata, :message, "unknown")
244299
Logger.info("Session cast #{session_id}: #{inspect(message)}")
245300
end
246301
end
247302

248-
defp handle_communication_event([:phoenix, :session_process, :start_error], _measurements, metadata, level) do
303+
defp handle_communication_event(
304+
[:phoenix, :session_process, :start_error],
305+
_measurements,
306+
metadata,
307+
level
308+
) do
249309
if should_log?(level, metadata) do
250310
session_id = Map.get(metadata, :session_id, "unknown")
251311
reason = Map.get(metadata, :reason, "unknown")
252312
Logger.error("Session start error #{session_id}: #{inspect(reason)}")
253313
end
254314
end
255315

256-
defp handle_communication_event([:phoenix, :session_process, :communication_error], _measurements, metadata, level) do
316+
defp handle_communication_event(
317+
[:phoenix, :session_process, :communication_error],
318+
_measurements,
319+
metadata,
320+
level
321+
) do
257322
if should_log?(level, metadata) do
258323
session_id = Map.get(metadata, :session_id, "unknown")
259324
reason = Map.get(metadata, :reason, "unknown")
260325
Logger.error("Session communication error #{session_id}: #{inspect(reason)}")
261326
end
262327
end
263328

264-
defp handle_cleanup_event([:phoenix, :session_process, :auto_cleanup], _measurements, metadata, level) do
329+
defp handle_cleanup_event(
330+
[:phoenix, :session_process, :auto_cleanup],
331+
_measurements,
332+
metadata,
333+
level
334+
) do
265335
if should_log?(level, metadata) do
266336
session_id = Map.get(metadata, :session_id, "unknown")
267337
Logger.debug("Auto-cleanup expired session: #{session_id}")
268338
end
269339
end
270340

271-
defp handle_cleanup_event([:phoenix, :session_process, :cleanup], _measurements, metadata, level) do
341+
defp handle_cleanup_event(
342+
[:phoenix, :session_process, :cleanup],
343+
_measurements,
344+
metadata,
345+
level
346+
) do
272347
if should_log?(level, metadata) do
273348
session_id = Map.get(metadata, :session_id, "unknown")
274349
Logger.debug("Cleanup session: #{session_id}")
275350
end
276351
end
277352

278-
defp handle_cleanup_event([:phoenix, :session_process, :cleanup_error], _measurements, metadata, level) do
353+
defp handle_cleanup_event(
354+
[:phoenix, :session_process, :cleanup_error],
355+
_measurements,
356+
metadata,
357+
level
358+
) do
279359
if should_log?(level, metadata) do
280360
session_id = Map.get(metadata, :session_id, "unknown")
281361
reason = Map.get(metadata, :reason, "unknown")

0 commit comments

Comments
 (0)