Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ExMachina.start/2: return a supervisor from Application callback #434

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

aeruder
Copy link
Contributor

@aeruder aeruder commented Oct 19, 2023

Description

ExMachina.start/2: return a supervisor from Application callback

We're actually returning the PID for an Agent rather than a
supervisor. In addition to being against the erlang callback spec:

    The function is to return {ok,Pid} or {ok,Pid,State}, where Pid
    is the pid of the top supervisor and State is any term. If
    omitted, State defaults to []. If the application is stopped
    later, State is passed to Module:prep_stop/1.

Some tools like phoenix live dashboard will send the
application supervisor supervisor-y messages and end up crashing the
root application tree.

    ** (FunctionClauseError) no function clause matching in Agent.Server.handle_call/3
        (elixir 1.14.5) lib/agent/server.ex:11: Agent.Server.handle_call(:which_children, {#PID<0.22073.22>, #Reference<0.3953567925.3593732098.137638>}, %{})
        (stdlib 3.17) gen_server.erl:721: :gen_server.try_handle_call/4
        (stdlib 3.17) gen_server.erl:750: :gen_server.handle_msg/6
        (stdlib 3.17) proc_lib.erl:226: :proc_lib.init_p_do_apply/3
    Last message (from #PID<0.22073.22>): :which_children

This change makes ExMachina return a supervisor with a single child -
the ExMachina.Sequence. Have also moved to use Agent to get the
child_spec/1 for free in ExMachina.Sequence.

This has been tested per the tool versions file (1.7.4 + otp 21).

 lib/ex_machina.ex          | 7 ++++++-
 lib/ex_machina/sequence.ex | 4 +++-
 2 files changed, 9 insertions(+), 2 deletions(-)

Before

ex_machina_crashing_in_livedashboard mov

Afterwards

image

Tests

test@742e43ef2efc:/ex_machina$ mix test
[...]
==> ex_machina
Compiling 16 files (.ex)
Generated ex_machina app
The database for ExMachina.TestRepo has been dropped
The database for ExMachina.TestRepo has been created
....................................................................................

Finished in 0.2 seconds
84 tests, 0 failures

Randomized with seed 422393
test@742e43ef2efc:/ex_machina$ elixir --version
Erlang/OTP 21 [erts-10.3.5.19] [source] [64-bit] [smp:5:5] [ds:5:5:10] [async-threads:1]

Elixir 1.7.4 (compiled with Erlang/OTP 21)

We're actually returning the PID for an Agent rather than a
supervisor.  In addition to being against the erlang callback spec:

    The function is to return {ok,Pid} or {ok,Pid,State}, where Pid
    is the pid of the top supervisor and State is any term. If
    omitted, State defaults to []. If the application is stopped
    later, State is passed to Module:prep_stop/1.

Some tools like phoenix live dashboard will send the application
supervisor supervisor-y messages and end up crashing the root
application tree.

    ** (FunctionClauseError) no function clause matching in Agent.Server.handle_call/3
        (elixir 1.14.5) lib/agent/server.ex:11: Agent.Server.handle_call(:which_children, {#PID<0.22073.22>, #Reference<0.3953567925.3593732098.137638>}, %{})
        (stdlib 3.17) gen_server.erl:721: :gen_server.try_handle_call/4
        (stdlib 3.17) gen_server.erl:750: :gen_server.handle_msg/6
        (stdlib 3.17) proc_lib.erl:226: :proc_lib.init_p_do_apply/3
    Last message (from #PID<0.22073.22>): :which_children

This change makes ExMachina return a supervisor with a single child -
the ExMachina.Sequence.  Have also moved to `use Agent` to get the
child_spec/1 for free in ExMachina.Sequence.

This has been tested per the tool versions file (1.7.4 + otp 21).
@doomspork doomspork requested a review from a team January 18, 2024 20:39
Copy link
Member

@doomspork doomspork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @aeruder! Now that ExMachina has been transferred to the BEAM Community we'll work on gettin these PRs incorporated and new releases flowing. Thanks again for the contribution!

@doomspork doomspork changed the title ExMachina.start/2: return a supervisor from Application callback feat: ExMachina.start/2: return a supervisor from Application callback Jan 18, 2024
@doomspork doomspork merged commit 664ad44 into beam-community:main Jan 18, 2024
1 check passed
doomspork pushed a commit that referenced this pull request Jan 18, 2024
#434)

We're actually returning the PID for an Agent rather than a
supervisor.  In addition to being against the erlang callback spec:

    The function is to return {ok,Pid} or {ok,Pid,State}, where Pid
    is the pid of the top supervisor and State is any term. If
    omitted, State defaults to []. If the application is stopped
    later, State is passed to Module:prep_stop/1.

Some tools like phoenix live dashboard will send the application
supervisor supervisor-y messages and end up crashing the root
application tree.

    ** (FunctionClauseError) no function clause matching in Agent.Server.handle_call/3
        (elixir 1.14.5) lib/agent/server.ex:11: Agent.Server.handle_call(:which_children, {#PID<0.22073.22>, #Reference<0.3953567925.3593732098.137638>}, %{})
        (stdlib 3.17) gen_server.erl:721: :gen_server.try_handle_call/4
        (stdlib 3.17) gen_server.erl:750: :gen_server.handle_msg/6
        (stdlib 3.17) proc_lib.erl:226: :proc_lib.init_p_do_apply/3
    Last message (from #PID<0.22073.22>): :which_children

This change makes ExMachina return a supervisor with a single child -
the ExMachina.Sequence.  Have also moved to `use Agent` to get the
child_spec/1 for free in ExMachina.Sequence.

This has been tested per the tool versions file (1.7.4 + otp 21).
doomspork pushed a commit that referenced this pull request Jan 18, 2024
#434)

We're actually returning the PID for an Agent rather than a
supervisor.  In addition to being against the erlang callback spec:

    The function is to return {ok,Pid} or {ok,Pid,State}, where Pid
    is the pid of the top supervisor and State is any term. If
    omitted, State defaults to []. If the application is stopped
    later, State is passed to Module:prep_stop/1.

Some tools like phoenix live dashboard will send the application
supervisor supervisor-y messages and end up crashing the root
application tree.

    ** (FunctionClauseError) no function clause matching in Agent.Server.handle_call/3
        (elixir 1.14.5) lib/agent/server.ex:11: Agent.Server.handle_call(:which_children, {#PID<0.22073.22>, #Reference<0.3953567925.3593732098.137638>}, %{})
        (stdlib 3.17) gen_server.erl:721: :gen_server.try_handle_call/4
        (stdlib 3.17) gen_server.erl:750: :gen_server.handle_msg/6
        (stdlib 3.17) proc_lib.erl:226: :proc_lib.init_p_do_apply/3
    Last message (from #PID<0.22073.22>): :which_children

This change makes ExMachina return a supervisor with a single child -
the ExMachina.Sequence.  Have also moved to `use Agent` to get the
child_spec/1 for free in ExMachina.Sequence.

This has been tested per the tool versions file (1.7.4 + otp 21).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants