noodlefrenzy / node-amqp10

amqp10 is a promise-based, AMQP 1.0 compliant node.js client
MIT License
134 stars 56 forks source link

fix(session): clear session states between reconnections #352

Closed pierreca closed 6 years ago

pierreca commented 6 years ago

Here's a (blunt) suggestion on how to clean sessions between reconnections - I'm not sure in which circumstances it's desirable to keep a session over multiple reconnections.

Without this some tests start failing with IoT Hub when trying to reattach links over "persisted" sessions.


This change is Reviewable

amarzavery commented 6 years ago

CI is failing

 1) Client #reconnect should wait for all existing sessions to be mapped before completing:

      Uncaught AssertionError: expected 0 to equal 1

      + expected - actual

      -0

      +1

      at assertEqual (node_modules/chai/lib/chai/core/assertions.js:487:12)

      at ctx.(anonymous function) (node_modules/chai/lib/chai/utils/addMethod.js:41:25)

      at doAsserterAsyncAndAddThen (node_modules/chai-as-promised/lib/chai-as-promised.js:293:29)

      at null.<anonymous> (node_modules/chai-as-promised/lib/chai-as-promised.js:252:17)

      at ctx.(anonymous function) [as equal] (node_modules/chai/lib/chai/utils/overwriteMethod.js:49:33)

      at AMQPClient.<anonymous> (test/unit/client.test.js:762:45)

      at Connection.<anonymous> (lib/amqp_client.js:9:9213)

      at Connection._processOpenFrame (lib/connection.js:9:19902)

      at Connection._receiveAny (lib/connection.js:9:15737)

      at Connection._receiveData (lib/connection.js:9:11006)

      at null.<anonymous> (lib/connection.js:9:22386)

      at Socket.<anonymous> (lib/transport/net_transport.js:9:1586)

      at Socket.<anonymous> (_stream_readable.js:765:14)

      at emitReadable_ (_stream_readable.js:427:10)

      at emitReadable (_stream_readable.js:423:5)

      at readableAddChunk (_stream_readable.js:166:9)

      at Socket.Readable.push (_stream_readable.js:128:10)

      at TCP.onread (net.js:529:21)
pierreca commented 6 years ago

wrong fix - new PR coming up from @princjef