danbarua / NEventSocket

A reactive FreeSwitch eventsocket library for Modern .Net
Mozilla Public License 2.0
73 stars 37 forks source link

Error on initialize socket #15

Closed SHAliakbari closed 9 years ago

SHAliakbari commented 9 years ago

Hi When user call FS and it open socket to manage the call , if call close befor init and calling Channels.Subscribe , it throw exception . I figure out problem and trace source . The errors occur in following classes :

  1. OutboundListener.Channels property . It Throw exception on "c.GetChannel().Result" . I put it in try cache and return null in exception and log the error, and check for null channel in callback . It fixed .
  2. Channel constructor . in Disposables subscribe . on "await eventSocket.Exit();" . I put it again in try cache . and log exception .
  3. In ObservableSocket . Observable.Defer on line "stream.ReadAsync(buffer, 0, buffer.Length);" . it throw error when socket closes before it . I put it on try cache and log error and also return null .

Now tell me , my works is OK or should change . I'm afraid of unexpected behaviors . So follow me in correct way , and also I use it in running application and the errors i mentioned closed the socket , and i should restart app . I'm also looking for exception handling which if socket close and dispose , it restart it self again .

sorry for my poor english

danbarua commented 9 years ago

I haven't written this into the README, but it's best practice to wrap all your subscription callbacks with a catch for OperationCancelledException - this is what you'll get when you write to a socket that's disconnected.

listener.Subscribe(async connection =>
{
  try
  {
    await connection.Connect();
    // do stuff with connection
  }
  catch(TaskCanceledException ex)
  {
    //connection closed, do any cleanup here
  }
});

This pattern should be used anywhere you are writing to a socket.

If you're getting anything else then that's a bug that we can look into.

danbarua commented 9 years ago

Also Socket.Exit() should swallow errors and clean up, so I'll look into that.

danbarua commented 9 years ago

I'm wondering whether we should throw a specific exception here, such as NEventSocketDisconnectedException instead of TaskCanceledException.

SHAliakbari commented 9 years ago

I use outband socket with pattern which exists in samples . and put listener and all of stuffs in try catch . My block is look like this :

try { LoggerExecutionWrapper nLog = (LoggerExecutionWrapper)LogProvider.GetLogger(GetType()); var listener = new OutboundListener(8086);

            listener.Channels.Subscribe(
                async channel =>
                {
                    if (channel == null)
                    {
                        nLog.Info("Channel Close before process ");
                        return;
                    }

                    try
                    {
                      . . . . .
                    }
                    catch (OperationCanceledException ex)
                    {
                        comment = "TaskCancelled - shutting down\r\n{0}".Fmt(ex.ToString());
                        comment += "\r\nChannel {0} is {1}\r\n".Fmt(channel.UUID, channel.Answered);
                        comment += Newtonsoft.Json.JsonConvert.SerializeObject(channel.lastEvent.Headers, Newtonsoft.Json.Formatting.Indented);

                });

            listener.Start();

            nLog.Info("Logger Start");

        }
        catch (Exception ex)
        {
            LoggerExecutionWrapper nLog = (LoggerExecutionWrapper)LogProvider.GetLogger(GetType());
            nLog.Error("Logger Exception : {0}".Fmt(ex.Message));
        }
danbarua commented 9 years ago

We need to clarify what the issue is here, are you seeing errors in the log output when there are errors (which is expected), or is your application crashing due to unhandled errors?

SHAliakbari commented 9 years ago

Actually in log files i see Dispose of some class and then nothing . It means my application crash due to unhandled error . I'm looking for a solution which handle all unexpected errors , and prevent my app from crashing . And I do my best ( put all stuff in try catch block ) but in some situations , the errors occur in other threads and it can not catch . As i mentioned in first post , I do some exception handling and my app didn't dead already . But i afraid of others unexpected errors.

danbarua commented 9 years ago

I've managed to reproduce (1) - it seems to block the connections subscription thread from receiving new connections - I am looking into this. re: 2 and 3 - These errors should be handled and logged for you, as long as you catch OperationCanceledException you should be ok.

danbarua commented 9 years ago

I've changed the Channels observable so that it does not terminate on a connection error, this should allow subsequent connections to succeed after a failed one. @SHAliakbari please can you try version 0.0.6.2 on NuGet and let me know if you are still having issues.

danbarua commented 9 years ago

@SHAliakbari please can you try version 0.6.3 which should handle connection errors better.

SHAliakbari commented 9 years ago

Hi Dan ,I will test it and tell you what happend .Also i made some changes on source . One of them is I change price lastEvent to Public . So i can use it on all over my code , not only event bases methods .

Date: Mon, 20 Jul 2015 04:55:23 -0700 From: notifications@github.com To: NEventSocket@noreply.github.com CC: amir.aliakbari@hotmail.com Subject: Re: [NEventSocket] Error on initialize socket (#15)

@SHAliakbari please can you try version 0.6.3 which should handle connection errors better.

— Reply to this email directly or view it on GitHub.

danbarua commented 9 years ago

I have surfaced some additional properties here which can be accessed via channel.Advanced.GetHeader() and channel.Advanced.GetVariable() https://github.com/danbarua/NEventSocket/blob/master/src/NEventSocket/Channels/BasicChannel.cs#L356