englercj / node-esl

FreeSWITCH ESL implementation for Node.js; implements the full Event Socket Library specified in: http://wiki.freeswitch.org/wiki/Esl
http://englercj.github.com/node-esl/
MIT License
170 stars 111 forks source link

shifted replies with outbound connection #72

Closed tleseney closed 5 years ago

tleseney commented 6 years ago

Hi,

I think there be be an issue with the outbound connection setup. When I run something like:

var server = new esl.Server({port: 8022, myevent: true}, () => console.log('server up'));
server.on('connection::ready', (conn, id) => {
  conn.sendRecv('linger', (result) => console.log(result));
});

The output is:

{ headers: 
   [ { name: 'Content-Type', value: 'command/reply' },
     { name: 'Reply-Text', value: '+OK Events Enabled' },

This looks that the result of the previous command. The initial outbound code in Connection.js reads:

this.send('connect');

this.once('esl::event::CHANNEL_DATA::**', function() {
            self.subscribe(self.reqEvents, function() {
                self.emit('esl::ready');
            });
        });

Since send is used for sending connect, there are no registered command/reply listener. As a result, the subsequent subscribe gets the connect result and then the linger gets the subscribe result. It seems sendRecv should be used, with the callback, it could probably be simplified as:

this.sendRecv('connect', function() {
            self.subscribe(self.reqEvents, function() {
                self.emit('esl::ready');
            });
        });

since the esl::event::CHANNEL_DATA is actually the reply to the connect command.

Regards,

Tom

englercj commented 6 years ago

Seems reasonable. I haven't worked on this lib for a while and don't have a freeswitch instance handy. If making this change works for you, feel free to put in a PR and I'll merge it.

tleseney commented 6 years ago

Thanks Chad for the feedback. I'll prepare a PR.

aalexgabi commented 6 years ago

I don't have a FreeSWITCH instance anymore either. I was wandering if it would take a long time to write an integration test for that using a Docker container with FreeSWITCH: https://hub.docker.com/r/bettervoice/freeswitch-container/

I think the same problem will be back for any PR so I thought a solution might be integration tests with a FreeSWITCH container.

englercj commented 6 years ago

Sounds great, I'd be happy to merge something that sets that up. Preferably with TravisCI.

chitkosarvesh commented 5 years ago

I think I am seeing similar issues. I am trying to run the "create_uuid" command on FreeSWITCH servers and I see almost comical responses from FreeSWITCH. I am not sure if it's my FS implementation or the node.js Library. If anyone can help me out or point me in the right direction?

Here's what I see:

{"command":"create_uuid","output":{"headers":[{"name":"Content-Type","value":"api/response"},{"name":"Content-Length","value":4}],"hPtr":null,"body":"+OK\n"}}

{"command":"create_uuid","output":{"headers":[{"name":"Content-Type","value":"api/response"},{"name":"Content-Length","value":10}],"hPtr":null,"body":"3126106146"}}

{"command":"create_uuid","output":{"headers":[{"name":"Content-Type","value":"api/response"},{"name":"Content-Length","value":10}],"hPtr":null,"body":"Eric Medor"}}

{"command":"create_uuid","output":{"headers":[{"name":"Content-Type","value":"api/response"},{"name":"Content-Length","value":2}],"hPtr":null,"body":"0\n"}}
englercj commented 5 years ago

A PR never went in for this, so AFAIK this issue still exists.

tleseney commented 5 years ago

Hi all,

I should be able to provide a PR in the weeks to come.

Thomas

Le 13 juin 2019 à 16:12, Chad Engler notifications@github.com a écrit :

A PR never went in for this, so AFAIK this issue still exists.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/englercj/node-esl/issues/72?email_source=notifications&email_token=AB3ZWROBNA2T2X6UKM3TZYTP2JIT3A5CNFSM4EZQCJG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXT2I6A#issuecomment-501720184, or mute the thread https://github.com/notifications/unsubscribe-auth/AB3ZWRJHOOZ6FRE4OPGGBULP2JIT3ANCNFSM4EZQCJGQ.