Crash report after gun_open_failed

[dzimi]

Hi !

When I open a connection to not responding endpoint I've got gun_open_failed error which is ok. But I've also got an Crash Report of shotgun fsm, because fsm returned {stop, gun_open_failed} in the init function which is, at the end, called by shotgun_sup.
Will this behavior change?
For me crash report should occur when something unexpected happen and this case is quite common.
What is your opinion about that?

[jfacorro]

@dzimi This is a good point you make.

An alternative I can think of to the current approach would be to add a new state called connecting and have the FSM's init function set that to the initial state. The problem I see with this is that when the user calls shotgun:open/[1, 2, 3] the function will return a pid(), but this is misleading, since that can be used to send messages, but the connection is not actually open.

There could be a shotgun:create/[1,2,3] and a shotgun:open/0 but that would force the user to make two function calls just to get things running.

Do you have additional alternatives to these? We should probably take a look at how gun handles this case since I'm sure we are not the only ones who have bumped into this issue 😛.

[dzimi]

My first idea to resolve this problem was to create receive clause in 'open' function, something like this

    open(Host, Port, Type, Opts) ->                                                                                                                                                                
     {ok, _} = supervisor:start_child(shotgun_sup, [Host, Port, Type, Opts]),                                                                                                                     
     receive                                                                                                                                                                                      
      {shotgun_open_resp, Resp} ->                                                                                                                                                               
       Resp                                                                                                                                                                                     
      after 30000 ->                                                                                                                                                                               
        erlang:error(shotgun_open_resp_timeout)                                                                                                                                              
     end.

In 'init' function try to open the connection and save result with pid of the client in state, return with {ok, State, 0}. Send asynchronous response in handle_info(timeout). This solution leaves current api unchanged. What do you think about it?

[dzimi]

I've forked your project and made a commit to fix this issue. Check out: dzimi@42995c4

What do you think about it?

[kennethlakin]

Ah! This explains why gun exits with 'normal' if gun:open fails, rather than actually providing a reasonable error value! If it's more valuable to prevent the crash reports than it is to distinguish between a failure to start due to gun:open timing out vs. gun:open encountering some other error, I'm fairly certain that shotgun:init/1 could be changed to return {stop, normal} in both of those cases.

However, we might be able to do both!

Could we modify shotgun_sup and shotgun:init/1 to get passed a new reference along with the PID of the function that's calling shotgun:open, so that shotgun:init/1 can return {stop, normal} if gun:open fails, but also pass back a message to the calling PID with more details about the failure? shotgun:open/4 might then look something like this (please pardon syntax errors)

open(Host, Port, Type, Opts) ->
    Ref = make_ref(),
    RxTimeout = 5000, %This is just for illustration.
    case supervisor:start_child(shotgun_sup, [Host, Port, Type, Opts, self(), Ref]) of
        {error, normal} ->
            receive {gun_open_error, Reason, Ref} ->
               {error, Reason}
             after RxTimeout
               {error, normal}
             end;
        Ret -> Ret
    end.

What do you think of that? Is it too clever?