Crash when removing endpoint

[dmorn]

When a child is removed from the RTC engine using the function, we're hit by this crash:

{"time":"2023-07-18T09:52:51.928Z","severity":"error","message":"GenServer #PID<0.1295.0> terminating\n** (Membrane.CallbackError) Callback handle_child_pad_removed/4 is not implemented in Membrane.RTC.Engine\n\n    (membrane_rtc_engine 0.15.0) Membrane.RTC.Engine.handle_child_pad_removed({:endpoint, \"streaming-endpoint\"}, {Membrane.Pad, :input, \"65d69082-5598-40c4-83ed-83f0d00e8486:0d97df5c-e8e1-4c19-b463-26e9cbf44386\"}, ...

The child is removed using the RTC.Engine.remove_endpoint(state.rtc_engine, peer_id) and once the remove_tracks notification is received we remove the children of the endpoint.

The crash happens on any version of membrane_core v0.12, RTC engine v0.15.

Implementing an empty handle_child_pad_removed/4 in the Membrane.RTC.Engine module solves the problem, but isn't that function supposed to be optional?

[dmorn]

Hey @Karolk99, was it fixed as of e407f06?

[dmorn]

The issue persists on master @Karolk99.

[FelonEkonom]

What has to be done, is to implement handle_child_pad_removed for input pads of WebRTC.Endpoint in RTC.Engine

[dmorn]

Yes this is what I've done in our fork. PR coming!

[dmorn]

Even though that callback is marked as optional, hence I would expect the fix to be implemented in membrane_core 😉

[mickel8]

Released as 0.15.1 🙂

[FelonEkonom]

Even though that callback is marked as optional, hence I would expect the fix to be implemented in membrane_core 😉

Callback handle_child_pad_removed/4 is optional because you can write a module, that implements @behaviour Membrane.Bin or Membrane.Pipeline without implementing handle_child_pad_removed/4 and it will compile without a warning. When you mark callback as optional in Elixir behaviour, it means, that you might, but you are not forced to implement it in the module that implements your behaviour.

We have improved the message in the error raising on try to call not implemented handle_child_pad_removed/4 in the upcoming release, but we won't make this callback not optional.