fix for race condition in issue #135

[lookfirst]

If we are publishing in a tight loop, the ack/error events may not be emitted in time to clear out the error event.

[lookfirst]

In addition, I just ran into this:

2013-02-11T18:57:53+00:00 app[emailAlert.1]: Unhandled connection error: FRAME_ERROR - type 110, all octets = <<>>: {frame_too_large,291859809,131064}

Which caused the heroku dyno to crash and start up again, but as it restarted...

2013-02-11T18:58:02+00:00 app[emailAlert.1]: exchange: ldexchange, queue: tasks
2013-02-11T18:58:02+00:00 app[emailAlert.1]: exchange: ldexchange, queue: fixImages
2013-02-11T18:58:02+00:00 app[emailAlert.1]: exchange: ldexchange, queue: emailAlert
2013-02-11T18:58:02+00:00 app[emailAlert.1]: exchange: ldexchange, queue: emailNewsletter
2013-02-11T18:58:02+00:00 app[emailAlert.1]: exchange: ldexchange, queue: chat
2013-02-11T18:58:02+00:00 app[emailAlert.1]: exchange: ldexchange, queue: apns
2013-02-11T18:58:02+00:00 app[emailAlert.1]: rabbit server: letsdate:cloudamp
2013-02-11T18:58:34+00:00 app[emailAlert.1]: (node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
2013-02-11T18:58:34+00:00 app[emailAlert.1]: Trace
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Connection.EventEmitter.addListener (events.js:175:15)
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Connection.EventEmitter.once (events.js:196:8)
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Connection.connect (/app/node_modules/amqp/amqp.js:1068:8)
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Connection.reconnect (/app/node_modules/amqp/amqp.js:1037:8)
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Object.backoff (/app/node_modules/amqp/amqp.js:875:16)
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Object.Proxy.callback.args.(anonymous function) [as _onTimeout] (/app/node_modules/nodetime/lib/core/proxy.js:131:20)
2013-02-11T18:58:34+00:00 app[emailAlert.1]:     at Timer.list.ontimeout (timers.js:101:19)
2013-02-11T18:58:42+00:00 app[emailAlert.1]: (node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
2013-02-11T18:58:42+00:00 app[emailAlert.1]: Trace
2013-02-11T18:58:42+00:00 app[emailAlert.1]:     at Connection.EventEmitter.addListener (events.js:175:15)
2013-02-11T18:58:42+00:00 app[emailAlert.1]:     at Connection.connect (/app/node_modules/amqp/amqp.js:1067:8)
2013-02-11T18:58:42+00:00 app[emailAlert.1]:     at Connection.reconnect (/app/node_modules/amqp/amqp.js:1037:8)
2013-02-11T18:58:42+00:00 app[emailAlert.1]:     at Object.backoff (/app/node_modules/amqp/amqp.js:875:16)
2013-02-11T18:58:42+00:00 app[emailAlert.1]:     at Object.Proxy.callback.args.(anonymous function) [as _onTimeout] (/app/node_modules/nodetime/lib/core/proxy.js:131:20)
2013-02-11T18:58:42+00:00 app[emailAlert.1]:     at Timer.list.ontimeout (timers.js:101:19)

Looks like the Connection code has a similar issue. Look for an update to this pull request. ;-)

[postwait]

Test case please.

[lookfirst]

There is zero documentation on how to even run your test suite, I'm not going to waste a day trying to figure it out. Sorry.

I also have no idea how to write a test that tests a race condition in eventing code. I could understand wanting a functional test, but this is not something that is testable other than to show that your existing tests shouldn't be broken after applying this code.

This code is running in production and used across ~100 dynos on heroku and I no longer see the stack traces after deploying it. Apply it or not. I'd rather not maintain my own fork, but if I need to, I will.

[zuk]

Ran into the FRAME_ERROR issue today as well when under high load. Would be nice to get this merged.

[compressed]

Agree. I'm running into this as well.

[vihang]

+1 Same Issue

[pdelanauze]

+1
ERR { '0': { [Error: FRAME_ERROR - type 101, all octets = <<>>: {frame_too_large,1935894629,131064}] code: 501 } }

[mohannad-b]

+1

[postwait]

Someone willing to whip up a short test case on this to make sure we don't have this regression in the future?

[pdelanauze]

I've tried getting the test suite to run many times to no avail, with instructions i'd gladly attempt writing a test case

[postwait]

The test suite expects a server running on the local machine.

make test

will run the rest suite... if you'd like to run a specific test alone:

node test/test-simple.js

If you'd like to use a server other than localhost:5762 then

make test SERVER=otherserver:port

or

node test/test-simple.js otherserver:port

The NODE_DEBUG_AMQP=1 environment variable can also be useful for debugging this stuff.

[postwait]

I guess when someone gets around to doing this again, they'll bother writing a test for it.

[lookfirst]

wow