Enhancement: Extend GarbageCollector
#766
Conversation
| def maybe_save_state!(%Trace{pid: pid, function: function, args: [_, _, socket]}) | ||
| when function in [:mount, :handle_params] do | ||
| do_save_initial_state!(pid, socket) | ||
| end |
There was a problem hiding this comment.
It's made as a preparation for better debugging dead LiveViews
hhubert6
force-pushed
the
763-extend-garbagecollector-to-utilize-removal-flags
branch
from
f2414fa to
165c0c1
Compare
|
|
||
| def maybe_save_state!(%Trace{pid: pid, function: function, args: [_, _, socket]}) | ||
| when function in [:mount, :handle_params] do | ||
| do_save_initial_state!(pid, socket) |
There was a problem hiding this comment.
I assume that this variant is used for LiveViews that dies immediately and we cannot fetch the state as usual via LiveViewDebug.liveview_state/1 because it may be already dead, right? The problem with this approach is that you don't have a list of LiveComponents in the state storage which theoretically may affect LiveViews that do not die immediately.
What will happen if someone triggers handle_params/3 that does not change any assigns? handle_params/3 in this case will be the last callback triggered in given LiveView (render/1 will not be triggered because assigns did not change). In such case components inspection may break
There was a problem hiding this comment.
I don't quite follow. If LiveView process dies so quickly that we can't fetch any information from it (e.g. LiveComponents) then we can't do much. But we can save socket from arguments of callbacks and show some information about this LiveView.
The problem with this approach is that you don't have a list of LiveComponents in the state storage which theoretically may affect LiveViews that do not die immediately.
In what way?
If the LiveView does not crash to the first connected render then most likely we will save it's state (full information about LiveComponents). There is still some chance that it will crash right after this render before processing our state fetch request but again we can't do much then.
|
|
||
| @impl true | ||
| def handle_info(%DebuggerTerminated{debugger_pid: debugger_pid}, state) do | ||
| def handle_info({:DOWN, _ref, :process, debugger_pid, _reason}, state) do |
There was a problem hiding this comment.
Why do we exactly change it? Using event is more flexible because you have to monitor in a single place only
There was a problem hiding this comment.
DebuggerTerminated was sent in terminate callback but turns out for some reason it's not triggered every time. For example when returning from debugger to active LiveViews it sometimes wasn't called so the event wasn't sent and the state of table_watcher was wrong. Using Process.monitor and listening for :DOWN is more reliable in this case.
| states_collected = GarbageCollectingActions.garbage_collect_states!(watched_pids, alive_pids) | ||
|
|
||
| if traces_collected or states_collected do | ||
| Bus.broadcast_event!(%GarbageCollected{}) |
There was a problem hiding this comment.
Why do we get rid of this event?
There was a problem hiding this comment.
It wasn't used anywhere and so when I refactored this part of code I didn't try to keep it if it's not even needed. We can always add it when there will be such a need.
No description provided.