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

Sometimes SockJSConnection.protocol is undefined #103

Closed
yurynix opened this issue Dec 10, 2012 · 6 comments
Closed

Sometimes SockJSConnection.protocol is undefined #103

yurynix opened this issue Dec 10, 2012 · 6 comments

Comments

@yurynix
Copy link

yurynix commented Dec 10, 2012

I'm on FreeBSD9, Node 0.9.3, sockjs 0.3.4

this._sockjs.on('connection', function(socket) {
...
        if ( socket.protocol === undefined ) {
            this.logger.log('warn', 'Got undefined protocol in socket: ' + util.inspect(socket));
        }
...
});

I'm getting a hit here, here's what in the log:

{"level":"warn","message":"Got undefined protocol in socket: { _session: 
   { session_id: 'z73v1uze',
     heartbeat_delay: 25000,
     disconnect_delay: 5000,
     prefix: '/myapp',
     send_buffer: [],
     is_closing: false,
     readyState: 1,
     timeout_cb: [Function],
     to_tref: 
      { _idleTimeout: 25000,
        _idlePrev: [Object],
        _idleNext: [Object],
        _when: 1355129374511,
        _onTimeout: [Function],
        _idleStart: 1355129349511 },
     connection: [Circular],
     emit_open: null,
     recv: 
      { request: [Object],
        response: [Object],
        options: [Object],
        curr_response_size: 2,
        thingy: [Object],
        thingy_end_cb: [Function],
        max_response_size: 131072,
        session: [Circular] } },
  id: 'd67493c3-b389-4312-9486-7d88fa5b55d4',
  headers: {},
  prefix: '/notifications',
  _events: { close: [Function], data: [Function] },
  connectTime: 1355129349511 }

No idea how to reproduce, seems to be very rare, one of 100,000 connections or so.

@majek
Copy link
Member

majek commented Dec 10, 2012

It may be caused by this line: https://github.com/sockjs/sockjs-node/blob/master/src/transport.coffee#L121

Ie: when polling connection comes in for a very short time, we might receive it, but not be able to get socket.* properties. In such case connection.protocol might not have been set.

@majek
Copy link
Member

majek commented Dec 10, 2012

@yurynix Can you apply 9cee2f3 and tell me if the problem still occurs?

@yurynix
Copy link
Author

yurynix commented Dec 10, 2012

@majek wow, what a quick response =)
I'll try it in a few days, now checking the connections issues in #99, want to let it run for a few days.

@yurynix
Copy link
Author

yurynix commented Dec 13, 2012

@majek If i'm not mistaken, the IF you've added only checks remoteAddress,
and does nothing regarding the protocol / other fields, am I missing something?

EDIT: Ohh, never mind, I see you've removed the return in the catch, it makes sense now. =)

@yurynix
Copy link
Author

yurynix commented Dec 13, 2012

@majek
I've run the patch, it passed now 300K connections, haven't seen the problem yet, so I'm guessing it fixed it, thanks =)

@majek
Copy link
Member

majek commented Dec 14, 2012

Released in 0.3.5.

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

No branches or pull requests

2 participants