From 52940573ded66bda2e193c94aefd9b33743ffef0 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 23 May 2024 17:34:58 +0200 Subject: [PATCH 1/4] Reshape put_in/update_in calls in crashgroups --- .../child_life_controller/crash_group_utils.ex | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/membrane/core/parent/child_life_controller/crash_group_utils.ex b/lib/membrane/core/parent/child_life_controller/crash_group_utils.ex index 8537f7d5c..715bc1143 100644 --- a/lib/membrane/core/parent/child_life_controller/crash_group_utils.ex +++ b/lib/membrane/core/parent/child_life_controller/crash_group_utils.ex @@ -16,8 +16,7 @@ defmodule Membrane.Core.Parent.ChildLifeController.CrashGroupUtils do def add_crash_group(group_name, mode, children, state) when not is_map_key(state.crash_groups, group_name) do put_in( - state, - [:crash_groups, group_name], + state.crash_groups[group_name], %CrashGroup{ name: group_name, mode: mode, @@ -28,7 +27,7 @@ defmodule Membrane.Core.Parent.ChildLifeController.CrashGroupUtils do @spec extend_crash_group(Child.group(), [Child.name()], Parent.state()) :: Parent.state() def extend_crash_group(group_name, children, state) do - update_in(state, [:crash_groups, group_name, :members], &(children ++ &1)) + update_in(state.crash_groups[group_name].members, &(children ++ &1)) end @spec get_child_crash_group(Child.name(), Parent.state()) :: {:ok, CrashGroup.t()} | :error @@ -44,7 +43,7 @@ defmodule Membrane.Core.Parent.ChildLifeController.CrashGroupUtils do # and we will not want to have it in :crash_group_members in the callback context in handle_crash_group_down/3, # so this child is removed from :members in crash group struct members = List.delete(group.members, child_name) - state = put_in(state, [:crash_groups, group.name, :members], members) + state = put_in(state.crash_groups[group.name].members, members) if group.detonating? and Enum.all?(members, &(not Map.has_key?(state.children, &1))) do cleanup_crash_group(group.name, state) @@ -83,8 +82,7 @@ defmodule Membrane.Core.Parent.ChildLifeController.CrashGroupUtils do end) update_in( - state, - [:crash_groups, group.name], + state.crash_groups[group.name], &%CrashGroup{ &1 | detonating?: true, @@ -96,7 +94,7 @@ defmodule Membrane.Core.Parent.ChildLifeController.CrashGroupUtils do defp cleanup_crash_group(group_name, state) do state = exec_handle_crash_group_down(group_name, state) - {_group, state} = pop_in(state, [:crash_groups, group_name]) + {_group, state} = pop_in(state.crash_groups[group_name]) state end @@ -104,7 +102,7 @@ defmodule Membrane.Core.Parent.ChildLifeController.CrashGroupUtils do group_name, state ) do - crash_group = get_in(state, [:crash_groups, group_name]) + crash_group = state.crash_groups[group_name] context_generator = &Component.context_from_state(&1, From a2d6202458fa4dd6d020c5a13eefcd82d714e9a6 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Mon, 27 May 2024 12:10:03 +0200 Subject: [PATCH 2/4] Add debug log about error in handling actions --- lib/membrane/core/callback_handler.ex | 18 ++++++++++++++++++ .../integration/links_validation_test.exs | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/membrane/core/callback_handler.ex b/lib/membrane/core/callback_handler.ex index c42a7400c..237d993b4 100644 --- a/lib/membrane/core/callback_handler.ex +++ b/lib/membrane/core/callback_handler.ex @@ -185,6 +185,8 @@ defmodule Membrane.Core.CallbackHandler do Error handling actions returned by callback #{inspect(state.module)}.#{callback} """) + log_debug_orginal_error(actions, e, __STACKTRACE__) + reraise e, __STACKTRACE__ end @@ -198,10 +200,26 @@ defmodule Membrane.Core.CallbackHandler do Error handling action #{inspect(action)} returned by callback #{inspect(state.module)}.#{callback} """) + log_debug_orginal_error(action, e, __STACKTRACE__) + reraise e, __STACKTRACE__ end end) handler_module.handle_end_of_actions(state) end + + defp log_debug_orginal_error(action_or_actions, error, stacktrace) do + action_or_actions = + if(is_list(action_or_actions), do: "actions ", else: "action ") <> + inspect(action_or_actions, limit: :infinity) + + Membrane.Logger.debug(""" + Error while handling #{action_or_actions} + + Orginal error: + #{inspect(error, pretty: true, limit: :infinity)} + #{Exception.format_stacktrace(stacktrace)} + """) + end end diff --git a/test/membrane/integration/links_validation_test.exs b/test/membrane/integration/links_validation_test.exs index 9f69e2f38..973f4921c 100644 --- a/test/membrane/integration/links_validation_test.exs +++ b/test/membrane/integration/links_validation_test.exs @@ -74,7 +74,7 @@ defmodule Membrane.LinksValidationTest do ] {:error, {{%Membrane.LinkError{}, _stackstrace}, _meta}} = - Pipeline.start_supervised(spec: spec) + Pipeline.start_supervised(spec: [spec, spec]) end test "dynamic pads" do From 1d37c15f660591b4392d1370c5a279aa1171e1c9 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Mon, 27 May 2024 15:27:21 +0200 Subject: [PATCH 3/4] Fix test --- test/membrane/integration/links_validation_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/membrane/integration/links_validation_test.exs b/test/membrane/integration/links_validation_test.exs index 973f4921c..9f69e2f38 100644 --- a/test/membrane/integration/links_validation_test.exs +++ b/test/membrane/integration/links_validation_test.exs @@ -74,7 +74,7 @@ defmodule Membrane.LinksValidationTest do ] {:error, {{%Membrane.LinkError{}, _stackstrace}, _meta}} = - Pipeline.start_supervised(spec: [spec, spec]) + Pipeline.start_supervised(spec: spec) end test "dynamic pads" do From 4ce63122595e3d980f2d20349f696c783abfef07 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Tue, 28 May 2024 10:32:46 +0200 Subject: [PATCH 4/4] Add comment --- lib/membrane/core/callback_handler.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/membrane/core/callback_handler.ex b/lib/membrane/core/callback_handler.ex index 237d993b4..186e50e04 100644 --- a/lib/membrane/core/callback_handler.ex +++ b/lib/membrane/core/callback_handler.ex @@ -209,6 +209,9 @@ defmodule Membrane.Core.CallbackHandler do handler_module.handle_end_of_actions(state) end + # We log it, because sometimes, for some reason, crashing process doesn't cause + # printing error logs on stderr, so this debug log allows us to get some info + # about what happened in case of process crash defp log_debug_orginal_error(action_or_actions, error, stacktrace) do action_or_actions = if(is_list(action_or_actions), do: "actions ", else: "action ") <>