Let advanced websocket module handle trapped exit messages #137

Closed
mlodzianck opened this Issue Jan 19, 2013 · 5 comments

Comments

Projects
None yet
3 participants
@mlodzianck

My use case:

  • client connects via websockets
  • callback module in 'advanced' mode is returned from out/1 function
  • callback module spawns some process
  • when spawned process exits abnormally whole gen_server process is terminated, client is disconnected, callback module is not called at all

How it should work:

  • client connects
  • callback's
handle_message(open, _)

is called (so it is related to #99 issue)

  • module can optionally set trap_exit flag to true in open callback
  • when linked process terminates
    handle_message({info,{'EXIT',FromPid,Reason}}, _)
    of callback module is called

Such feature gives more control over child processes and enables better error handling.

@vinoski

This comment has been minimized.

Show comment
Hide comment
@vinoski

vinoski Jan 21, 2013

Collaborator

Any chance you could supply some code to serve as a test case for this?

Collaborator

vinoski commented Jan 21, 2013

Any chance you could supply some code to serve as a test case for this?

@capflam

This comment has been minimized.

Show comment
Hide comment
@capflam

capflam Jan 21, 2013

Collaborator

Hi,

I'm working on the websockets refactoring. With this new version, it will be possible to catch exit messages from linked processes. I almost finished my work on it, but for now, you could find some details by reading the comments of the issue #99 and the commit logs of the websockets branch.

Collaborator

capflam commented Jan 21, 2013

Hi,

I'm working on the websockets refactoring. With this new version, it will be possible to catch exit messages from linked processes. I almost finished my work on it, but for now, you could find some details by reading the comments of the issue #99 and the commit logs of the websockets branch.

@mlodzianck

This comment has been minimized.

Show comment
Hide comment
@mlodzianck

mlodzianck Jan 21, 2013

@capflam
Thanks! I see this 167fdb8 commit resolves my issue. So I'm waiting for HEAD merge and new release.
Little OT question: maybe is it good idea to define behaviour for websockets callback module, what is your opinion?

@vinoski
Very quick example

Callback module

-module(buggy_ws).
-export([handle_message/2,out/1]).
-include("yaws_api.hrl").

out(_A)->
        {websocket, ?MODULE, [{callback, {advanced, []}}]}.

handle_message(#ws_frame_info{}, _) ->
    io:format("Got frame, spawning buggy process~n"),
    buggy_proc:start_link(),
    {noreply, []};

handle_message(Message, _) ->
    io:format("Got some non-frame message ~p ~n",[Message]),
        %%Here I would like to receive trapped exits, but from commit log is see that there 
        %%will be dedicated handle_info/2 function in callback module
    {noreply, []}.

Spawned process

-module(buggy_proc).
-behaviour(gen_server).
-export([init/1, handle_info/2, terminate/2,start_link/0]).

start_link() ->
    gen_server:start(?MODULE, [], []).
init([]) ->
    io:format("Buggy process will crash in 3 sec.~n"),
    {ok, [], 3000}.

handle_info(timeout, _) ->
   {stop,no_normal_exit, []}.

terminate(Reson,_) ->
    io:format("Terminating buggy proc, reason = ~p~n", [Reson]).

@capflam
Thanks! I see this 167fdb8 commit resolves my issue. So I'm waiting for HEAD merge and new release.
Little OT question: maybe is it good idea to define behaviour for websockets callback module, what is your opinion?

@vinoski
Very quick example

Callback module

-module(buggy_ws).
-export([handle_message/2,out/1]).
-include("yaws_api.hrl").

out(_A)->
        {websocket, ?MODULE, [{callback, {advanced, []}}]}.

handle_message(#ws_frame_info{}, _) ->
    io:format("Got frame, spawning buggy process~n"),
    buggy_proc:start_link(),
    {noreply, []};

handle_message(Message, _) ->
    io:format("Got some non-frame message ~p ~n",[Message]),
        %%Here I would like to receive trapped exits, but from commit log is see that there 
        %%will be dedicated handle_info/2 function in callback module
    {noreply, []}.

Spawned process

-module(buggy_proc).
-behaviour(gen_server).
-export([init/1, handle_info/2, terminate/2,start_link/0]).

start_link() ->
    gen_server:start(?MODULE, [], []).
init([]) ->
    io:format("Buggy process will crash in 3 sec.~n"),
    {ok, [], 3000}.

handle_info(timeout, _) ->
   {stop,no_normal_exit, []}.

terminate(Reson,_) ->
    io:format("Terminating buggy proc, reason = ~p~n", [Reson]).
@vinoski

This comment has been minimized.

Show comment
Hide comment
@vinoski

vinoski Jan 21, 2013

Collaborator

Thanks. It's always helpful to have test cases we can add to our regression test suite.

Regarding the idea of a websockets callback behaviour, I considered that in the past and I think it might be a good idea.

Collaborator

vinoski commented Jan 21, 2013

Thanks. It's always helpful to have test cases we can add to our regression test suite.

Regarding the idea of a websockets callback behaviour, I considered that in the past and I think it might be a good idea.

@capflam

This comment has been minimized.

Show comment
Hide comment
@capflam

capflam Feb 18, 2013

Collaborator

The changes about WebSockets were pushed in the master branch. Feel free to test it. the release 1.96 is on the way.

Collaborator

capflam commented Feb 18, 2013

The changes about WebSockets were pushed in the master branch. Feel free to test it. the release 1.96 is on the way.

@capflam capflam closed this Feb 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment