Stop ingesting packets after the client has errored

[mcollina]

Fixed #21.

@cefn please test and see if it solves your issue.

[cefn]

Indeed this branch helps our production test suite to proceed beyond the error I reported.

We're now having much subtler issues when running with Aedes (versus Mosquitto) which only appear when interactively testing. I'm investigating if it's to do with failing to honour the Last Will and Testament - what's the status of LWT for Aedes?

It seems that the Last Will and Testament is being written to the topic tree before the normal trigger event (when the browser is shut down) or potentially something is forcing the client to be disconnected and hence correctly triggering the LWT (which is somehow swallowed as an error in our production code).

On the plus side I can see that the topic tree reported by mosquitto_sub -t '#' -v from a Aedes production test run and a Mosquitto production test run is so similar that I'm having to investigate in some detail what is the difference which makes one work and the other not, which is a pretty good sign. Given the complexity of the production code, there's still the chance it is some kind of race condition given the timings of the brokers responding could be different. Will let you know if I can recreate in a simpler test suite.

[cefn]

Forgot to mention for reference that Mosca does satisfy our integration testing suite, but just super-slow owing to performance issues as detailed at moscajs/mosca#385 which which we couldn't easily hide from users.

Aedes is much, much more performant in our integration testing. It's not as fast as the newly-patched version of Mosquitto (Ral helped me eliminate the 200ms round-trip delay, at the cost of burning some cycles in poll loops), but is fast enough in end-to-end behaviour that we could certainly use it in production and has much better potential for provisioning and administering access-control lists from within our node server logic, so we could take the hit on raw performance in favour of these other aspects I think.

For those reasons I'm very interested to keep sweating Aedes until it satisfies our integration testing suite, assuming I can figure out the issue we're hitting right now.

[mcollina]

@cefn are you running with some form of keepalive? Might be some form of max-mapper/websocket-stream#69.

BTW, I would be really happy if you can shelve off some ms from here as well.

[cefn]

I assume that given our production code is Node and V8 in Chrome, running MQTT.js then we should be golden (https://www.npmjs.com/package/mqtt#client and max-mapper/websocket-stream#69 (comment) suggest keepalive would be 10 seconds by default in MQTT.js without any special configuration I think).

Generally I have had trouble getting meaningful performance profiling out of V8 so far (so much indirection in the libraries we were using before) but may be better now we've removed almost all our promise and stream code in the latest iteration. With a bit of luck I'll be able to pull out the Aedes functions which run the longest and explore any marginal improvements. Our tests are frequently disadvantaged by a cold-start without optimised hot paths so I'm not sure there's any real performance problem yet.